From mboxrd@z Thu Jan  1 00:00:00 1970
From: Simon Glass <sjg@chromium.org>
Date: Sun, 31 May 2020 08:08:53 -0600
Subject: [PATCH v1 1/3] drivers: reset: Handle gracefully NULL pointers
In-Reply-To: <20200527171205.rjmzduwd6bfwhdtj@ti.com>
References: <CAPnjgZ1iL+wKYr7aBsSBBPEWsPzq0qha2cHrst1qP1Kjes7dXw@mail.gmail.com>
 <20200527171205.rjmzduwd6bfwhdtj@ti.com>
Message-ID: <CAPnjgZ3bLdSSffoO2Uef5U348_UuNZ6TJUmx1+ocwrXSLaMG2g@mail.gmail.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Hi Pratyush,

On Wed, 27 May 2020 at 11:12, Pratyush Yadav <p.yadav@ti.com> wrote:
>
> Hi Simon,
>
> On 29/10/19 07:48PM, Simon Glass wrote:
> > Hi Jean-Jacques,
> >
> > On Mon, 30 Sep 2019 at 08:31, Jean-Jacques Hiblot <jjhiblot@ti.com> wrote:
> > >
> > > Prepare the way for a managed reset API by handling NULL pointers without
> > > crashing nor failing.
> > >
> > > Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
> > > ---
> > >
> > >  drivers/reset/reset-uclass.c | 30 +++++++++++++++++-------------
> > >  1 file changed, 17 insertions(+), 13 deletions(-)
> >
> > Same comment here about code size / Kconfig option.
>
> A lot of the changes below are ternary operators. I'm not sure how to
> put them behind a Kconfig option to reduce code size. Do you want me to
> change the NULL check to a separate if() block to put behind an #ifdef?
>
> One way of doing this is like in this patch [0]. The other would be to
> wrap individual if statements in #ifdef, which tends to hurt
> readability.

Well for this I would really favour:

  int reset_request(struct reset_ctl *reset_ctl)
  {
     struct reset_ops *ops;

if (!result_ctl)
    return 0;

ops = reset_dev_ops(reset_ctl->dev);

But why allow result_ctl to be NULL? You should explain that in your
commit message.

> > -     struct reset_ops *ops = reset_dev_ops(reset_ctl->dev);
> > +     struct reset_ops *ops = reset_dev_ops(reset_ctl);
> >
> >       debug("%s(reset_ctl=%p)\n", __func__, reset_ctl);
> >
> > -     return ops->request(reset_ctl);
> > +     return ops->request ? ops->request(reset_ctl) : 0;
> >  }
> >
>
> [0] https://patchwork.ozlabs.org/project/uboot/patch/20191001115130.18886-2-jjhiblot at ti.com/

[..]

Regards,
Simon