Re: [PATCH v2 RESEND] pwm: Add CLPS711X PWM support

[Mark Rutland <mark.rutland@arm.com>]

On Tue, Feb 25, 2014 at 03:50:32PM +0000, Arnd Bergmann wrote:
> On Tuesday 25 February 2014 19:47:57 Alexander Shiyan wrote:
> > Вторник, 25 февраля 2014, 16:33 +01:00 от Arnd Bergmann <arnd@arndb.de>:
> > > On Tuesday 25 February 2014 19:27:47 Alexander Shiyan wrote:
> => > 
> > > We really want to avoid wildcards in compatible strings. Can you call this
> > > "cirrus,cs89712-pwm" to match the first SoC that came with this hardware?
> > > Obviously if there was some chip before that (I'm not familiar with the
> > > model numbers), use that instead.
> > > 
> > > You can either list all chips you know that have this in the driver,
> > > or you use "cirrus,cs89712-pwm" as the fallback, and use the name of
> > > the SoC you have as the more specific string.
> > 
> > It seems that in this case we will need to modify the compatibility string
> > for other drivers that are already available in the kernel...
> 
> Ah, right. I missed the binding for gpio and serial going in.
> 
> DT maintainers, any suggestion on how we should proceed here?
> 
> AFAICT, clps711x platform support is still work-in-progress, so we don't
> have any upstream users to worry about yet, but changing them is still
> going to be slightly messy.

When allocating any new compatible strings, as Arnd says, we should
avoid wildcards as they're usually far too encompassing and end up
causing more trouble than they're worth.

In this case how problematic are the wildcard strings?

I assume we still have specific strings earlier in any compatible list
in any case? If not, and there are possible differences, that should be
fixed right away.

We have a few options:

a) Update the docs only.

   Note in the docs that "cirrus,clps711x-$UNIT" means anything
   compatible with the $UNIT in the cs89712. This may be
   counter-intuitive, and if a new clps711x platform comes out with an
   incompatible $UNIT it should omit "cirrus,clps711x-$UNIT" from its
   compatible list, but otherwise no harm done.

b) Deprecate the existing string.
  
   Add "cirrus,cs89712-$UNIT" to the binding docs and driver. Mark
   "cirrus,clps711x-$UNIT" as deprecated in the binding document.
   Replace "cirrus,clps711x-$UNIT" with "cirrus,cs89712-$UNIT" in all
   DTs.

   This will mean new DTBs won't work with an older kernel, but as
   support is a work in progress that might not matter. Old DTBs will
   continue to function.

   Iff you can guarantee that the old string is not possibly being used
   by anyone, it can be dropped from the driver. If not it has to remain
   (though can be commented to be deprecated), so that old DTBs
   function. It's probably best to leave it there.
  
c) Deprecate, maintaining (forwards) compatibility.

   As above, but rather than replacing "cirrus,clps711x-$UNIT" with
   "cirrus,cs89712-$UNIT" in DTs, place "cirrus,cs89712-$UNIT" before
   "cirrus,clps711x-$UNIT" in DTs. This allows new DTBs to work with
   older kernels too. Depending on what level of support you have in
   mainline currently this may or may not be an important consideration.

Cheers,
Mark.