Re: [PATCH v7 5/5] iio: magnetometer: Add mmc5633 sensor

Andy Shevchenko
2025-10-30T07:51:16Z

On Wed, Oct 29, 2025 at 11:54:39AM -0400, Frank Li wrote:
> On Tue, Oct 28, 2025 at 12:50:55PM +0200, Andy Shevchenko wrote:
> > On Mon, Oct 27, 2025 at 04:08:33PM -0400, Frank Li wrote:

...

> > > +static const struct {
> > > + int val;
> > > + int val2;
> >
> > No need. Just
> >
> > > +} mmc5633_samp_freq[] = {
>
> There are some place will like
>
> if (mmc5633_samp_freq[i][0] == val &&
> mmc5633_samp_freq[i][1] == val2)
>
> previous
> if (mmc5633_samp_freq[i].val == val &&
> mmc5633_samp_freq[i].val2 == val2)
>
> Previous version seem have better readablity. But it is not big deal, if
> you like, I change to [0][1].

It's not my preference, it's how all but this driver do in IIO, it's about
consistency of the style / patterns.

...

> > struct i3c_device doesn't have a name, does it?
>
> It has name, but it is hexnumber (VID+PID), like 0-4a20000f000.
> So use friend/readable name here.

I think it's better to use the name provided by the subsystem. This way we may
guarantee the unique one. The hard coded values potentially might collide
(imagine some I³C driver for the very similar chip, let's say, in hwmon).

--
With Best Regards,
Andy Shevchenko