On Fri, Sep 18, 2020 at 04:26:06PM +0200, Rasmus Villemoes wrote:
> Somewhere between v2020.04 and v2020.07 the mpc8xxx_spi driver broke,
> I'm guessing due to this hunk
>
> @@ -559,6 +560,8 @@ int dm_gpio_set_dir_flags(struct gpio_desc *desc, ulong
> flags)
> if (ret)
> return ret;
>
> + /* combine the requested flags (for IN/OUT) and the descriptor flags
> */
> + flags |= desc->flags;
> ret = _dm_gpio_set_dir_flags(desc, flags);
>
> from commit 695e5fd5469a ("gpio: update dir_flags management"). But
> the blame is mostly on the driver itself which seems rather confused:
> The chip select gpios are requested with GPIOD_ACTIVE_LOW, but then in
> each activate/deactivate, dm_gpio_set_dir_flags() is called with
> merely GPIOD_IS_OUT, and then the driver call set_value(0) for
> activate.
>
> That used to work, but with the above hunk, the ACTIVE_LOW setting
> from the request becomes persistent, so the gpio driver ends up being
> asked to set the value to 1 in mpc8xxx_spi_cs_activate().
>
> So drop the dm_gpio_set_dir_flags() calls in the activate/deactivate
> functions, and use a value of 1 to mean "logically enabled".
>
> Ideally, I think we should also drop the GPIOD_ACTIVE_LOW from the
> request and make it up to the list of gpio cs in DT to indicate
> whether that CS is enabled when driven low (as is of course usually
> the case), but that requires changing
> arch/powerpc/dts/gdsys/gazerbeam-base.dtsi among others, and I don't
> have that hardware to test on. I have, however, tested our
> own (mpc8309-based) hardware with this change, and I have also tested
> that removing the GPIOD_ACTIVE_LOW from the request and updating our
> DT as
>
> - gpios = <&spisel 0 0>;
> + gpios = <&spisel 0 GPIO_ACTIVE_LOW>;
>
> still works.
>
> Signed-off-by: Rasmus Villemoes <rasmus.villem...@prevas.dk>Applied to u-boot/master, thanks! -- Tom
signature.asc
Description: PGP signature
