lkml.org 
[lkml]   [2021]   [Nov]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v10 3/5] iio: adc: Add Xilinx AMS driver
On Thu, Nov 18, 2021 at 02:14:41PM +0000, Anand Ashok Dumbre wrote:
> > From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > Sent: Wednesday 17 November 2021 8:03 PM
> > On Wed, Nov 17, 2021 at 04:10:26PM +0000, Anand Ashok Dumbre wrote:

...

> > > + ret = ams_read_vcc_reg(ams, chan->address, val);
> > > + if (ret) {
> > > + mutex_unlock(&ams->lock);
> > > + return -EINVAL;
> >
> > Shadowed error code.
>
> I don’t understand.

You get an error code in ret variable, but you always return -EINVAL, why?

> > > + }

...

> > default: ?
>
> This is limited by hw bits.
> For default I will use the default scan_index value.
> Is that ok?

I think so, you know hardware better, you decide what to put there.

...

> > > + if (ret || reg > (AMS_PL_MAX_EXT_CHANNEL + 30))
> >
> > Too many parentheses.
>
> Is it a good practice to not have parantheses around (AMS_PL_MAX_EXT_CHANNEL + 30) ?

It's minor thing, up to you.

> > > + continue;

...

> > > + dev_chan_size = array_size((size_t)num_channels, sizeof(struct
> > iio_chan_spec));
> > > + if (dev_chan_size == SIZE_MAX)
> > > + return -EINVAL;
> >
> > Why is it needed now?
> >
> > > + dev_channels = devm_kcalloc(&pdev->dev, (size_t)num_channels,
> >
> > Why casting?
> >
> > > + sizeof(struct iio_chan_spec), GFP_KERNEL);
> >
> > sizeof(*dev_channels)
> >
> > > + if (!dev_channels) {
> > > + ret = -ENOMEM;
> > > + goto free_mem;
> > > + }
> >
> > > + memcpy(dev_channels, ams_channels,
> > > + sizeof(*ams_channels) * num_channels);
> >
> > Hmm... according to the code the num_channels can be less than or equal to
> > num_chan. Hence, what you should use is the devm_krealloc_array().
> >
> > static inline void *devm_krealloc_aray(...) {
> > ...see how krealloc_array() is defined...
> > }
> >
> > No need to copy memory again.
>
> Will take a look.

--
With Best Regards,
Andy Shevchenko


\
 
 \ /
  Last update: 2021-11-18 16:43    [W:0.097 / U:1.056 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site