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@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/barebox
