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

From: Robin Murphy <robin.murphy@arm.com>
To: Andrew Lunn <andrew@lunn.ch>
Cc: Linus Walleij <linus.walleij@linaro.org>,
	Mark Rutland <mark.rutland@arm.com>,
	"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
	<devicetree@vger.kernel.org>, Jason Cooper <jason@lakedaemon.net>,
	"open list:GPIO SUBSYSTEM" <linux-gpio@vger.kernel.org>,
	Gregory Clement <gregory.clement@bootlin.com>,
	Russell King - ARM Linux admin <linux@armlinux.org.uk>,
	linux-pwm@vger.kernel.org,
	Bartosz Golaszewski <bgolaszewski@baylibre.com>,
	Rob Herring <robh+dt@kernel.org>,
	Thierry Reding <thierry.reding@gmail.com>,
	Uwe Kleine-Konig <u.kleine-koenig@pengutronix.de>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>,
	Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
Subject: Re: [PATCH RFC 0/6] PWM fan support on Clearfog gt8k
Date: Thu, 16 Apr 2020 17:20:05 +0100	[thread overview]
Message-ID: <628cde2b-0e1b-4c07-4321-8b23d0b34384@arm.com> (raw)
In-Reply-To: <20200416144231.GP657811@lunn.ch>

On 2020-04-16 3:42 pm, Andrew Lunn wrote:
> On Thu, Apr 16, 2020 at 03:37:40PM +0100, Robin Murphy wrote:
>> On 2020-04-16 2:50 pm, Andrew Lunn wrote:
>> [...]
>>> Clocking with Marvell devices has always been interesting. Core IP
>>> like this gets reused between different generations of SoCs. The
>>> original Orion5x had no clock control at all. Latter SoCs have had
>>> more and more complex clock trees. So care has to be taken to not
>>> change old behaviour when adding support for new clocks.
>>
>> FWIW, that sounds like a good argument for encoding the clock requirements
>> of each variant in the of_match_data, so the driver doesn't have to simply
>> trust the DT and hope.
> 
> Hi Robin
> 
> It is not really hope. It is very obvious when it is wrong, the whole
> machine stops dead when you are missing a clock. Very simple to test.

Heh, that's still what I meant - the driver hopes that carrying on will 
be OK, and the end user is left to pick up the pieces when it isn't ;)

Obviously that's less of an issue when said end-user is a kernel 
developer making a controlled change during SoC bringup, but perhaps 
more so for an eager inexperienced hacker cobbling together DTS 
fragments to convince a distro kernel to boot on some embedded device 
(even as the former I know I've had enough frustration from unclocked 
registers showing up in unexpected places - the best is when connecting 
an external debugger to see where it's stuck happens to enable the 
offending clock and let the CPU progress to somewhere else by the time 
it actually halts...). If it's possible to make a driver robust enough 
to fail cleanly, isn't that always nicer to debug?

Robin.