Aw: Re: [PATCH v1] ARM: Rockchip: Add rk3568 BananaPi R2 Pro board support

[Frank Wunderlich <frank-w@public-files.de>]

Hi,
> Gesendet: Montag, 24. Januar 2022 um 09:55 Uhr
> Von: "Sascha Hauer" <s.hauer@pengutronix.de>

> > +static bool machine_is_rk3568_r2pro = false;
> > +
> > +static int rk3568_r2pro_probe(struct device_d *dev)
> > +{
> > +	enum bootsource bootsource = bootsource_get();
> > +	int instance = bootsource_get_instance();
> > +
> > +	barebox_set_model("RK3568 R2PRO");
> > +	barebox_set_hostname("rk3568-r2pro");
> > +	machine_is_rk3568_r2pro = true;
>
> Maybe add a bpi to the hostname, model, variable names and instead drop
> the rk3568?

i can do...

> > +	if (!IS_ENABLED(CONFIG_AIODEV))
> > +		return 0;
> > +
> > +	if (!machine_is_rk3568_r2pro)
> > +		return 0;
> > +
> > +	hwid_chan = aiochannel_by_name("aiodev0.in_value1_mV");
> > +	if (IS_ERR(hwid_chan)) {
> > +		ret = PTR_ERR(hwid_chan);
> > +		goto err_hwid;
> > +	}
> > +
> > +	ret = aiochannel_get_value(hwid_chan, &hwid_voltage);
> > +	if (ret)
> > +		goto err_hwid;
> > +
> > +	pr_info("hwid_voltage: %d\n", hwid_voltage);
>
> The voltage is not really interesting. This should be a pr_debug.

ok, i change this

> > +
> > +	if (hwid_voltage == 1800)
> > +		hwid = "V00";
> > +	else
> > +		hwid = "unknown";
>
> Have you verified this board really encodes the hardware revision using
> this adc channel?

yes, in shematics it is named SARADC_VIN1_HW_ID (which should map the in_value1_mV), but i have only this one prototype for testing.

> > diff --git a/arch/arm/dts/rk3568-bpi-r2-pro.dts b/arch/arm/dts/rk3568-bpi-r2-pro.dts
>
> I saw this device tree looks different from the one you sent for kernel
> inclusion. This is not a problem now, but once the kernel dts is
> upstream it will show up in dts/ in the barebox tree. We then usually
> include the upstream dts from the barebox board dts, so that only the
> barebox specific additions are in the barebox dts. Given that it would
> be nice if you could minimize the differences now already, so that later
> inclusion of the upstream dts becomes easier.

ok, we can wait for kernel dts to be merged, but most stuff in kernel dts will be unused in barebox. so i started with the evb one which is enough for barebox atm. Only added the leds.

regards Frank

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox