Re: [PATCH v1 01/24] spi: mpc512x: prepare clocks before enabling them

[Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>]

[-- Attachment #1.1: Type: text/plain, Size: 2415 bytes --]

On Wed, Jul 17, 2013 at 04:26:28PM +0200, Gerhard Sittig wrote:
> On Wed, Jul 17, 2013 at 13:07 +0100, Mark Brown wrote:

> > This is a pretty long e-mail.  It'd probably have taken less time to
> > fix the issues than to reply to the e-mail...  but anyway.

> Not quite.  Please consider that careful submission is as
> expensive as thorough review is, and that a lot of work is done
> before v1 gets submitted, which you may not always notice from
> looking at a concentrated series that may no longer suggest how
> much it took to get there (especially when prepared carefully).

This is rather undermined the more time and effort gets spent pushing
back against doing trival fixes, of course...  besides, the issues here
are all really simple to fix and test.  It's not a major or risky
rewrite of the driver or anything like that - most of this can be tested
by just probing the driver.

> As mentioned before I did not question the need to fix issues as
> they get identified.  I just asked whether reworking the serial

OK, so send patches then.

> The initial COMMON_CLK implementation in part 09/24 registers
> clkdev items for compatibility during migration.  The string
> isn't greppable since it's constructed by the preprocessor in the
> MCLK data setup, see the MCLK_SETUP_DATA_PSC() macro.

Ugh, right - it didn't show up in searches due to being hidden by the
macro.

> Now let me see how I can improve the SPI driver with regard to
> overall clock handling and beyond mere operation by coincidence
> in the absence of errors.  But please understand that I don't
> want to stall the common clock support for a whole platform just
> because some of the drivers it needs to touch to keep them
> operational have other issues that weren't addressed yet, while
> these issues aren't introduced with the series but preceed it.

Again, you're not being asked to implement substantial new functionality
here.  From my point of view I can't test this at all so I'm looking at
code that's obviously not good for the standard clock API and wondering
if it even works or how robust it's going to be going forward as the
common clock code changes which makes me relucatant to say it'll be OK.

The fact that you're switching over to use generic code is itself a
reason to make sure that the API usage is sane, dodgy API usage against
open coded clock implementations is less risky than against the common
code.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 192 bytes --]

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss