Hi Krzysztof
On Tue, Mar 01, 2022 at 01:34:28PM +0100, Krzysztof Hałasa wrote:
> Jacopo Mondi <jacopo@xxxxxxxxxx> 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
