Re: [PATCH RFC 0/6] PWM fan support on Clearfog gt8k

[Russell King - ARM Linux admin <linux@armlinux.org.uk>]

On Thu, Apr 16, 2020 at 02:08:36PM +0200, Linus Walleij wrote:
> On Thu, Apr 16, 2020 at 10:14 AM Russell King - ARM Linux admin
> <linux@armlinux.org.uk> wrote:
> > On Thu, Apr 16, 2020 at 09:51:37AM +0200, Linus Walleij wrote:
> 
> > > The gpio changes all look fine to me +/- fixes for review comments.
> >
> > I think Uwe is incorrect for his GPIO comments; the clock is only
> > optional on A8040.  We know this because A8040 has worked fine
> > without PWM support without the clock, whereas for Armada 370,
> > the driver has hard-failed if the clock is not present.
> 
> It's fine. You are running the hardware and it should work for you.
> I usually go by the IETF motto "rough consensus and running code".
> 
> > About the only change I would make is to move the check introduced
> > in patch 2 into patch 3 instead, inside the MVEBU_PWM_SOC_VARIANT_A8K
> > case, so that deferring for the clock works (which is necessary for
> > the PWM driver to be useful.)
> 
> OK let's go with this.

Well, it turns out to not be particularly nice to do that.  The best
I can come up with is:

        mvchip->clk = devm_clk_get(&pdev->dev, NULL);
        if (soc_variant->pwm && IS_ENABLED(CONFIG_PWM) &&
            mvchip->clk == ERR_PTR(-EPROBE_DEFER))
                return -EPROBE_DEFER;

Doing it in mvebu_pwm_probe() means that we have to deal with unwinding
the very complex probing (tearing down all the interrupt functionality
and GPIO stuff) which the driver currently does not do, even on failure.

Is this a shoddy driver that doesn't clean up after itself...

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 10.2Mbps down 587kbps up