| Message ID | 20170512132227.24916-10-romain.perier@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, May 12, 2017 at 03:22:25PM +0200, Romain Perier wrote: > This commit adds a compatible string for everest,es8323. This is an > audio codec that is compatible with es8328 and can be found for example > on the Firefly-RK3288 board. If it is compatible with the es8328, then that should be a fallback and you don't need the driver change. > > Signed-off-by: Romain Perier <romain.perier@collabora.com> > --- > > Changes in v3: None > Changes in v2: None > > Documentation/devicetree/bindings/sound/es8328.txt | 5 ++++- > sound/soc/codecs/es8328-i2c.c | 2 ++ > 2 files changed, 6 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/sound/es8328.txt b/Documentation/devicetree/bindings/sound/es8328.txt > index 33fbf058c997..86b6d6e99732 100644 > --- a/Documentation/devicetree/bindings/sound/es8328.txt > +++ b/Documentation/devicetree/bindings/sound/es8328.txt > @@ -4,7 +4,10 @@ This device supports both I2C and SPI. > > Required properties: > > - - compatible : Should be "everest,es8328" or "everest,es8388" > + - compatible : Should be one of the following: > + - "everest,es8323" So this would be '"everest,es8323", "everest,es8328"' instead. > + - "everest,es8328" > + - "everest,es8388" > - DVDD-supply : Regulator providing digital core supply voltage 1.8 - 3.6V > - AVDD-supply : Regulator providing analog supply voltage 3.3V > - PVDD-supply : Regulator providing digital IO supply voltage 1.8 - 3.6V
On Mon, May 15, 2017 at 04:41:38PM -0500, Rob Herring wrote: > On Fri, May 12, 2017 at 03:22:25PM +0200, Romain Perier wrote: > > This commit adds a compatible string for everest,es8323. This is an > > audio codec that is compatible with es8328 and can be found for example > > on the Firefly-RK3288 board. > If it is compatible with the es8328, then that should be a fallback and > you don't need the driver change. While people don't strictly need the driver change it doesn't do any harm either and encourages people to get the information into the DT that it's a different chip. Thinking about it it might be good to have a way for something to validate if fallback compatibles are being listed when they should - the drivers could provide the information fairly easily I guess, or it could go into the binding docs once we have a schema format. Even if it doesn't currently make a difference to software I'd rather get the information in there for mixed signal devices like audio CODECs - it's not that unknown to find later that supposedly register identical chips have some differences in the analog which we want to care about.
Hello, Le 16/05/2017 à 13:18, Mark Brown a écrit : > On Mon, May 15, 2017 at 04:41:38PM -0500, Rob Herring wrote: >> On Fri, May 12, 2017 at 03:22:25PM +0200, Romain Perier wrote: >>> This commit adds a compatible string for everest,es8323. This is an >>> audio codec that is compatible with es8328 and can be found for example >>> on the Firefly-RK3288 board. >> If it is compatible with the es8328, then that should be a fallback and >> you don't need the driver change. > While people don't strictly need the driver change it doesn't do any > harm either and encourages people to get the information into the DT > that it's a different chip. Thinking about it it might be good to have > a way for something to validate if fallback compatibles are being listed > when they should - the drivers could provide the information fairly > easily I guess, or it could go into the binding docs once we have a > schema format. > > Even if it doesn't currently make a difference to software I'd rather > get the information in there for mixed signal devices like audio CODECs > - it's not that unknown to find later that supposedly register identical > chips have some differences in the analog which we want to care about. So, what should I do for this patch, finally ? fallback or driver change ? Thanks, Romain
Le 19/05/2017 à 08:56, Romain Perier a écrit : > Hello, > > > Le 16/05/2017 à 13:18, Mark Brown a écrit : >> On Mon, May 15, 2017 at 04:41:38PM -0500, Rob Herring wrote: >>> On Fri, May 12, 2017 at 03:22:25PM +0200, Romain Perier wrote: >>>> This commit adds a compatible string for everest,es8323. This is an >>>> audio codec that is compatible with es8328 and can be found for example >>>> on the Firefly-RK3288 board. >>> If it is compatible with the es8328, then that should be a fallback and >>> you don't need the driver change. >> While people don't strictly need the driver change it doesn't do any >> harm either and encourages people to get the information into the DT >> that it's a different chip. Thinking about it it might be good to have >> a way for something to validate if fallback compatibles are being listed >> when they should - the drivers could provide the information fairly >> easily I guess, or it could go into the binding docs once we have a >> schema format. >> >> Even if it doesn't currently make a difference to software I'd rather >> get the information in there for mixed signal devices like audio CODECs >> - it's not that unknown to find later that supposedly register identical >> chips have some differences in the analog which we want to care about. > So, what should I do for this patch, finally ? fallback or driver change ? > > Thanks, > Romain To be honest, the doc is not really clear about this, both codecs seem compatible but I think that's preferable to have a driver change for this, just in case (Suppose that we discover a difference later...) Romain
On Fri, May 19, 2017 at 1:56 AM, Romain Perier <romain.perier@collabora.com> wrote: > Hello, > > > Le 16/05/2017 à 13:18, Mark Brown a écrit : >> On Mon, May 15, 2017 at 04:41:38PM -0500, Rob Herring wrote: >>> On Fri, May 12, 2017 at 03:22:25PM +0200, Romain Perier wrote: >>>> This commit adds a compatible string for everest,es8323. This is an >>>> audio codec that is compatible with es8328 and can be found for example >>>> on the Firefly-RK3288 board. >>> If it is compatible with the es8328, then that should be a fallback and >>> you don't need the driver change. >> While people don't strictly need the driver change it doesn't do any >> harm either and encourages people to get the information into the DT >> that it's a different chip. Thinking about it it might be good to have >> a way for something to validate if fallback compatibles are being listed >> when they should - the drivers could provide the information fairly >> easily I guess, or it could go into the binding docs once we have a >> schema format. >> >> Even if it doesn't currently make a difference to software I'd rather >> get the information in there for mixed signal devices like audio CODECs >> - it's not that unknown to find later that supposedly register identical >> chips have some differences in the analog which we want to care about. > So, what should I do for this patch, finally ? fallback or driver change ? Well, you could do both. However, there's not much point if you do the driver change. So: Acked-by: Rob Herring <robh@kernel.org>
diff --git a/Documentation/devicetree/bindings/sound/es8328.txt b/Documentation/devicetree/bindings/sound/es8328.txt index 33fbf058c997..86b6d6e99732 100644 --- a/Documentation/devicetree/bindings/sound/es8328.txt +++ b/Documentation/devicetree/bindings/sound/es8328.txt @@ -4,7 +4,10 @@ This device supports both I2C and SPI. Required properties: - - compatible : Should be "everest,es8328" or "everest,es8388" + - compatible : Should be one of the following: + - "everest,es8323" + - "everest,es8328" + - "everest,es8388" - DVDD-supply : Regulator providing digital core supply voltage 1.8 - 3.6V - AVDD-supply : Regulator providing analog supply voltage 3.3V - PVDD-supply : Regulator providing digital IO supply voltage 1.8 - 3.6V diff --git a/sound/soc/codecs/es8328-i2c.c b/sound/soc/codecs/es8328-i2c.c index 318ab28c5351..be3f03c35137 100644 --- a/sound/soc/codecs/es8328-i2c.c +++ b/sound/soc/codecs/es8328-i2c.c @@ -19,6 +19,7 @@ #include "es8328.h" static const struct i2c_device_id es8328_id[] = { + { "es8323", 0 }, { "es8328", 0 }, { "es8388", 0 }, { } @@ -26,6 +27,7 @@ static const struct i2c_device_id es8328_id[] = { MODULE_DEVICE_TABLE(i2c, es8328_id); static const struct of_device_id es8328_of_match[] = { + { .compatible = "everest,es8323", }, { .compatible = "everest,es8328", }, { .compatible = "everest,es8388", }, { }
This commit adds a compatible string for everest,es8323. This is an audio codec that is compatible with es8328 and can be found for example on the Firefly-RK3288 board. Signed-off-by: Romain Perier <romain.perier@collabora.com> --- Changes in v3: None Changes in v2: None Documentation/devicetree/bindings/sound/es8328.txt | 5 ++++- sound/soc/codecs/es8328-i2c.c | 2 ++ 2 files changed, 6 insertions(+), 1 deletion(-)