Hi Marek,

On 21 September 2018 at 16:59, Marek Vasut <marek.va...@gmail.com> wrote:
> The code fails to check if ops is non-NULL before using it's members.
> Add the missing check and while at it, flip the condition to make it
> more obvious what is actually happening.
>
> Signed-off-by: Marek Vasut <marek.vasut+rene...@gmail.com>
> Cc: Simon Glass <s...@chromium.org>
> Cc: Tom Rini <tr...@konsulko.com>
> ---
>  drivers/pci/pci-uclass.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c
> index eb118f3496..de523a76ad 100644
> --- a/drivers/pci/pci-uclass.c
> +++ b/drivers/pci/pci-uclass.c
> @@ -241,9 +241,9 @@ int pci_bus_write_config(struct udevice *bus, pci_dev_t 
> bdf, int offset,
>         struct dm_pci_ops *ops;
>
>         ops = pci_get_ops(bus);
> -       if (!ops->write_config)
> -               return -ENOSYS;
> -       return ops->write_config(bus, bdf, offset, value, size);
> +       if (ops && ops->write_config)
> +               return ops->write_config(bus, bdf, offset, value, size);

I'd like to avoid this if possible, since it bloats code. If you don't
provide operations you are on your own!

Ideas:
- add it behind DEBUG
- check it once in the uclass when binding - e.g. in uclass_add() -
and print a warning?

I have pushed back pretty hard against people adding checks for things
which should not be true in normal systems. Partly it is just for the
confusion it adds, partly for efficiency. Perhaps we should document
the pre-conditions that DM guarantees somewhere?

Regards,
Simon
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Reply via email to