Re: [PATCH v2 3/3] iio: magnetometer: add driver for QST QMC5883L Sensor

[Siratul Islam]

June 17, 2026 at 1:13 PM, "Andy Shevchenko" <andriy.shevchenko@xxxxxxxxx mailto:andriy.shevchenko@xxxxxxxxx?to=%22Andy%20Shevchenko%22%20%3Candriy.shevchenko%40intel.com%3E > wrote:


> 
> On Tue, Jun 16, 2026 at 05:49:39PM +0600, Siratul Islam wrote:
> 
> > 
> > Add driver for the QST QMC5883L 3-Axis Magnetic Sensor
> >  connected via i2c.
> > 
> ...
> 
> > 
> > +enum qmc5883l_chan {
> >  + QMC5883L_AXIS_X,
> >  + QMC5883L_AXIS_Y,
> >  + QMC5883L_AXIS_Z
> > 
> Leave trailing comma, it's not a dedicated terminator.
> 
> > 
> > +};
I generally have a habit of not adding a comma at the end as it reminds me of javascript/json. But sometimes I add it to help clang-format. Jonathan suggested me to remove the commas at "terminating" entries. So I took the opportunity to remove them everywhere. Apparently it was a wrong decision. Thnaks for pointing this out, I'll make it a new habit. As always I'm learning a lot from these reviews.
> ...
> 
> > 
> > +
> >  + return 0;
> >  +}
> > 
> ...
> Ideally this should have a comment with a reference to the datasheet where this
> delay is specified. Otherwise a comment why this exact value has been chosen.
> 
I have a comment where I define this, about this value being from the datasheet. I'll update it to specify where though.
> > 
> > + fsleep(QMC5883L_PORT_US);
...
> 
> > 
> > +};
> > 
> -- 
> With Best Regards,
> Andy Shevchenko
>

Thanks
Sirat