Re: [PATCH v8 2/2] On Semi AR0521 sensor driver

[Jacopo Mondi]

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