Re: [PATCH v3 2/2] gpio: ixp4xx: Handle clock output on pin 14 and 15

[Linus Walleij <linus.walleij@xxxxxxxxxx>]

Oops found some unaddressed feedback and outliers since
september.

Time to catch up.

On Mon, Sep 25, 2023 at 9:19 AM Andy Shevchenko <andy@xxxxxxxxxx> wrote:

> Given that cover letter implicitly explains why not PPS,
> Reviewed-by: Andy Shevchenko <andy@xxxxxxxxxx>

Thanks!

> Can be optimized this way (not insisting, though):

It's nice, I changed it.

> I'm wondering if it's fine to have them both to be cleared if not defined?
> I.o.w. does it meant that appearance of one of the properties (to be set)
> implies the other (to be not set)?

I'm just cautious, there may be some components that require this
clocking that we don't know about, and that may be a reason why
they are enabled by e.g. RedBoot (the bootloader) at power on
and never touched.

The entire support is reverse-engineered so... we don't know exactly
what is going on. Would be easier if we has source for everything :/

>                         val &= ~(IXP4XX_GPCLK_MUX14 | IXP4XX_GPCLK_MUX15);
>                         val &= ~IXP4XX_GPCLK_CLK0_MASK;
>                         val &= ~IXP4XX_GPCLK_CLK1_MASK;
>                 }
>         }
>
>         if (clk_14) {
>
>                 val |= (0 << IXP4XX_GPCLK_CLK0DC_SHIFT);
>
> Wondering why you simply can't replace this...
>
>                 val |= (1 << IXP4XX_GPCLK_CLK0TC_SHIFT);
>                 val |= IXP4XX_GPCLK_MUX14;
>         }
>
>         if (clk_15) {
>                 val |= (0 << IXP4XX_GPCLK_CLK1DC_SHIFT);
>
> ...and this by a comment?

Yup, fixed it.

Yours,
Linus Walleij