Re: [PATCH v2 4/8] serial_ns16550: handle default reg-io-width

[Sascha Hauer <s.hauer@pengutronix.de>]

On Thu, Dec 13, 2018 at 04:56:46PM +0100, Oleksij Rempel wrote:
> Am 13.12.18 um 08:44 schrieb Rouven Czerwinski:
> > -	if (!of_property_read_u32(np, "reg-io-width", &width))
> > -		switch (width) {
> > -		case 1:
> > -			priv->read_reg = ns16550_read_reg_mmio_8;
> > -			priv->write_reg = ns16550_write_reg_mmio_8;
> > -			break;
> > -		case 2:
> > -			priv->read_reg = ns16550_read_reg_mmio_16;
> > -			priv->write_reg = ns16550_write_reg_mmio_16;
> > -			break;
> > -		case 4:
> > -			if (of_device_is_big_endian(np)) {
> > -				priv->read_reg = ns16550_read_reg_mmio_32be;
> > -				priv->write_reg = ns16550_write_reg_mmio_32be;
> > -			} else {
> > -				priv->read_reg = ns16550_read_reg_mmio_32;
> > -				priv->write_reg = ns16550_write_reg_mmio_32;
> > -			}
> > -			break;
> > -		default:
> > -			dev_err(dev, "unsupported reg-io-width (%d)\n",
> > -				width);
> > +	of_property_read_u32(np, "reg-io-width", &width);
> 
> i think it is not good to drop error handling completely. We may fail in
> different ways:
> static const void *of_find_property_value_of_size(const struct
> device_node *np,
> 			const char *propname, u32 len)
> {
> 	struct property *prop = of_find_property(np, propname, NULL);
> 	const void *value;
> 
> 	if (!prop)
> 		return ERR_PTR(-EINVAL);
> 	value = of_property_get_value(prop);
> 	if (!value)
> 		return ERR_PTR(-ENODATA);
> 	if (len > prop->length)
> 		return ERR_PTR(-EOVERFLOW);

Although we could differenciate between these different values and
return an error I think initializing a variable with a default and let
of_property_read_* overwrite it without further error checking is common
sense now, you can find hundreds of hits in the Linux Kernel.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

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