Messages in this thread | | | Date | Tue, 1 Mar 2022 15:30:44 +0100 | From | Jacopo Mondi <> | Subject | Re: [PATCH v8 2/2] On Semi AR0521 sensor driver |
| |
Hi Krzysztof
On Tue, Mar 01, 2022 at 01:34:28PM +0100, Krzysztof Hałasa wrote: > Jacopo Mondi <jacopo@jmondi.org> writes: > > > In subject > > > > "media: i2c:" > > > > Same for 1/2 where permutation of "media: dt-bindings: i2c:" are used > > when adding bindings for media i2c drivers. > > You know, it's rather hard to know all these tiny requirements. Let > alone get them right all the time. >
I usually rely on what git log suggests me if in doubt.
> >> + pm_runtime_set_active(&client->dev); > >> + pm_runtime_enable(&client->dev); > >> + pm_runtime_idle(&client->dev); > > > > Do you have an _idle() callback ? This seems a no-op to me, or am I > > mistaken ? (runtime_pm is still cryptic to me sometimes) > > Do you mean only the pm_runtime_idle()? Sakari just requested I add it. >
Ok then
> > Can't you just remove power_on() if it's not needed ? > > It wouldn't work without CONFIG_PM then, would it? >
True that. And an #ifdef is maybe undesirable.
Although, if with idle you can power-down the chip after probe, I'm fine with that
> > There still are a few checkpatch warnings which might be worth > > considering but nothing huge. > > Do you mean this? > > WARNING: Block comments use * on subsequent lines > #411: FILE: drivers/media/i2c/ar0521.c:351: > + /* Reset gain, the sensor may produce all white pixels without > + this */ > > WARNING: Block comments use a trailing */ on a separate line > #411: FILE: drivers/media/i2c/ar0521.c:351: > + this */ > > ... which are about a single comment which I had to wrap to fit in 80 > columns, hardly a block comment by my standards. Not to mention Linus > saying...
The warning suggests to go for
/* * This when a comment * spans on multiple lines */
instead of
/* Going for this style when a comment is on multiple lines */
> -- > Krzysztof "Chris" Hałasa > > Sieć Badawcza Łukasiewicz > Przemysłowy Instytut Automatyki i Pomiarów PIAP > Al. Jerozolimskie 202, 02-486 Warszawa
| |