Re: [PATCH v2] ASoC: cs42888: Add codec driver support
>> On Mar 11, 2014, at 13:46, "Mark Brown" wrote: >> >> On Tue, Mar 11, 2014 at 04:55:58PM +0100, Lars-Peter Clausen wrote: >> On 03/11/2014 04:41 PM, Brian Austin wrote: > >>> So WRT the CS42888, this is one device in a series of 2 devices that are >>> register compatible with the only difference being that the CS42488 has 2 >>> extra ADC's. Same die and same DeviceID. > >>> Would it make more sense to submit the driver with the extra ADC's to make >>> it feature complete for both devices? Wouldn't be odd to have 2 drivers that >>> do the same for 98% of the code. > >> This should be in one driver. But support for the second device can >> always be added in a follow up patch. > > Yes, we can add support later. It might be helpful to name the driver > with some generic name I guess. CS42xx8 would work. We have a CS42448 and the CS42888. The difference is the 2 extra ADC's on the CS42448. I can add those later. I still would like to go over some of the v2 code though before a v3 is sent. Thanks Brian -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] ASoC: cs42888: Add codec driver support
Hi Brian, On Wed, Mar 12, 2014 at 02:05:00AM +, Austin, Brian wrote: > > > >> On Mar 11, 2014, at 13:46, "Mark Brown" wrote: > >> > >> On Tue, Mar 11, 2014 at 04:55:58PM +0100, Lars-Peter Clausen wrote: > >> On 03/11/2014 04:41 PM, Brian Austin wrote: > > > >>> So WRT the CS42888, this is one device in a series of 2 devices that are > >>> register compatible with the only difference being that the CS42488 has 2 > >>> extra ADC's. Same die and same DeviceID. > > > >>> Would it make more sense to submit the driver with the extra ADC's to make > >>> it feature complete for both devices? Wouldn't be odd to have 2 drivers > >>> that > >>> do the same for 98% of the code. > > > >> This should be in one driver. But support for the second device can > >> always be added in a follow up patch. > > > > Yes, we can add support later. It might be helpful to name the driver > > with some generic name I guess. > CS42xx8 would work. > > We have a CS42448 and the CS42888. The difference is the 2 extra ADC's on the > CS42448. I can add those later. > > I still would like to go over some of the v2 code though before a v3 is sent. Actually I've already sent v3 right after this v2 due to some doc updating. Anyway, I'd love to wait for your comments against the patch before I send v3. Thank you, Nicolin -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] ASoC: cs42888: Add codec driver support
On Tue, Mar 11, 2014 at 06:46:06PM +, Mark Brown wrote: > On Tue, Mar 11, 2014 at 04:55:58PM +0100, Lars-Peter Clausen wrote: > > On 03/11/2014 04:41 PM, Brian Austin wrote: > > > >So WRT the CS42888, this is one device in a series of 2 devices that are > > >register compatible with the only difference being that the CS42488 has 2 > > >extra ADC's. Same die and same DeviceID. > > > >Would it make more sense to submit the driver with the extra ADC's to make > > >it feature complete for both devices? Wouldn't be odd to have 2 drivers > > >that > > >do the same for 98% of the code. > > > This should be in one driver. But support for the second device can > > always be added in a follow up patch. > > Yes, we can add support later. It might be helpful to name the driver > with some generic name I guess. I didn't notice there's a sister model for CS42888. But good to know. And I think I should try cs42x88.c as the driver name, which makes sense even if we have another sister here. Or Brian may give me a more official one? It would be better if I also put those CS42488's support into the driver but I'll start a long vacation from tomorrow. So if I am not able to do that for CS42488's parts in the short time, I'll send the CS42888 part first by the end of the day and we can add CS42488 later. Thanks all, Nicolin Chen -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] ASoC: cs42888: Add codec driver support
On Tue, Mar 11, 2014 at 04:55:58PM +0100, Lars-Peter Clausen wrote: > On 03/11/2014 04:41 PM, Brian Austin wrote: > >So WRT the CS42888, this is one device in a series of 2 devices that are > >register compatible with the only difference being that the CS42488 has 2 > >extra ADC's. Same die and same DeviceID. > >Would it make more sense to submit the driver with the extra ADC's to make > >it feature complete for both devices? Wouldn't be odd to have 2 drivers that > >do the same for 98% of the code. > This should be in one driver. But support for the second device can > always be added in a follow up patch. Yes, we can add support later. It might be helpful to name the driver with some generic name I guess. signature.asc Description: Digital signature
Re: [PATCH v2] ASoC: cs42888: Add codec driver support
On 03/11/2014 04:41 PM, Brian Austin wrote: On Tue, 11 Mar 2014, Nicolin Chen wrote: This patch adds support for the Cirrus Logic CS42888 Audio CODEC that has four 24-bit A/D and eight 24-bit D/A converters. [ CS42888 supports both I2C and SPI control ports. As initial patch, this patch only adds the support for I2C. ] Signed-off-by: Nicolin Chen --- So WRT the CS42888, this is one device in a series of 2 devices that are register compatible with the only difference being that the CS42488 has 2 extra ADC's. Same die and same DeviceID. Would it make more sense to submit the driver with the extra ADC's to make it feature complete for both devices? Wouldn't be odd to have 2 drivers that do the same for 98% of the code. This should be in one driver. But support for the second device can always be added in a follow up patch. - Lars -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] ASoC: cs42888: Add codec driver support
On Tue, 11 Mar 2014, Nicolin Chen wrote: This patch adds support for the Cirrus Logic CS42888 Audio CODEC that has four 24-bit A/D and eight 24-bit D/A converters. [ CS42888 supports both I2C and SPI control ports. As initial patch, this patch only adds the support for I2C. ] Signed-off-by: Nicolin Chen --- So WRT the CS42888, this is one device in a series of 2 devices that are register compatible with the only difference being that the CS42488 has 2 extra ADC's. Same die and same DeviceID. Would it make more sense to submit the driver with the extra ADC's to make it feature complete for both devices? Wouldn't be odd to have 2 drivers that do the same for 98% of the code. Thoughts? Brian -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] ASoC: cs42888: Add codec driver support
On Tue, Mar 11, 2014 at 11:30:53AM +, Mark Rutland wrote: > On Tue, Mar 11, 2014 at 11:19:51AM +, Nicolin Chen wrote: > > This patch adds support for the Cirrus Logic CS42888 Audio CODEC that > > has four 24-bit A/D and eight 24-bit D/A converters. > > > > [ CS42888 supports both I2C and SPI control ports. As initial patch, > > this patch only adds the support for I2C. ] > > > > Signed-off-by: Nicolin Chen > > --- > > > > Changelog > > v2: > > * Separated driver into I2C and main part drivers. > > * Added error out if failed to get clock. > > * Enabled Auto-mute function as default. > > * Disabled runtime-pm in remove(). > > * Added return value checking for regcache_sync(). > > * Fixed some trival parts like 'const' as Lars mentioned. > > > > .../devicetree/bindings/sound/cs42888.txt | 27 ++ > > sound/soc/codecs/Kconfig | 10 + > > sound/soc/codecs/Makefile | 4 + > > sound/soc/codecs/cs42888-i2c.c | 68 +++ > > sound/soc/codecs/cs42888.c | 536 > > + > > sound/soc/codecs/cs42888.h | 213 > > 6 files changed, 858 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/sound/cs42888.txt > > create mode 100644 sound/soc/codecs/cs42888-i2c.c > > create mode 100644 sound/soc/codecs/cs42888.c > > create mode 100644 sound/soc/codecs/cs42888.h > > > > diff --git a/Documentation/devicetree/bindings/sound/cs42888.txt > > b/Documentation/devicetree/bindings/sound/cs42888.txt > > new file mode 100644 > > index 000..7736527 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/sound/cs42888.txt > > @@ -0,0 +1,27 @@ > > +CS42888 audio CODEC > > + > > +Required properties: > > + > > + - compatible : "cirrus,cs42888" > > + > > + - reg : the I2C address of the device for I2C > > + > > + - clocks : phandle to the clock source for MCLK > > + > > + - clock-names : must contain "mclk". > > If you use clock-names, please define the clocks property in terms of > clock-names to avoid redundancy. e.g. > > - clocks: a list of phandles + clock-specifiers, one for each entry in > clock-names. > > It makes the relationship between the two clearer, and makes it far > easier to alter the binding in future if required. Thank you for the quick reply. Will fix it in a minute. Best regards, Nicolin Chen -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] ASoC: cs42888: Add codec driver support
On Tue, Mar 11, 2014 at 11:19:51AM +, Nicolin Chen wrote: > This patch adds support for the Cirrus Logic CS42888 Audio CODEC that > has four 24-bit A/D and eight 24-bit D/A converters. > > [ CS42888 supports both I2C and SPI control ports. As initial patch, > this patch only adds the support for I2C. ] > > Signed-off-by: Nicolin Chen > --- > > Changelog > v2: > * Separated driver into I2C and main part drivers. > * Added error out if failed to get clock. > * Enabled Auto-mute function as default. > * Disabled runtime-pm in remove(). > * Added return value checking for regcache_sync(). > * Fixed some trival parts like 'const' as Lars mentioned. > > .../devicetree/bindings/sound/cs42888.txt | 27 ++ > sound/soc/codecs/Kconfig | 10 + > sound/soc/codecs/Makefile | 4 + > sound/soc/codecs/cs42888-i2c.c | 68 +++ > sound/soc/codecs/cs42888.c | 536 > + > sound/soc/codecs/cs42888.h | 213 > 6 files changed, 858 insertions(+) > create mode 100644 Documentation/devicetree/bindings/sound/cs42888.txt > create mode 100644 sound/soc/codecs/cs42888-i2c.c > create mode 100644 sound/soc/codecs/cs42888.c > create mode 100644 sound/soc/codecs/cs42888.h > > diff --git a/Documentation/devicetree/bindings/sound/cs42888.txt > b/Documentation/devicetree/bindings/sound/cs42888.txt > new file mode 100644 > index 000..7736527 > --- /dev/null > +++ b/Documentation/devicetree/bindings/sound/cs42888.txt > @@ -0,0 +1,27 @@ > +CS42888 audio CODEC > + > +Required properties: > + > + - compatible : "cirrus,cs42888" > + > + - reg : the I2C address of the device for I2C > + > + - clocks : phandle to the clock source for MCLK > + > + - clock-names : must contain "mclk". If you use clock-names, please define the clocks property in terms of clock-names to avoid redundancy. e.g. - clocks: a list of phandles + clock-specifiers, one for each entry in clock-names. It makes the relationship between the two clearer, and makes it far easier to alter the binding in future if required. Cheers, Mark. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] ASoC: cs42888: Add codec driver support
On Tue, Mar 11, 2014 at 11:19:51AM +, Nicolin Chen wrote: This patch adds support for the Cirrus Logic CS42888 Audio CODEC that has four 24-bit A/D and eight 24-bit D/A converters. [ CS42888 supports both I2C and SPI control ports. As initial patch, this patch only adds the support for I2C. ] Signed-off-by: Nicolin Chen guangyu.c...@freescale.com --- Changelog v2: * Separated driver into I2C and main part drivers. * Added error out if failed to get clock. * Enabled Auto-mute function as default. * Disabled runtime-pm in remove(). * Added return value checking for regcache_sync(). * Fixed some trival parts like 'const' as Lars mentioned. .../devicetree/bindings/sound/cs42888.txt | 27 ++ sound/soc/codecs/Kconfig | 10 + sound/soc/codecs/Makefile | 4 + sound/soc/codecs/cs42888-i2c.c | 68 +++ sound/soc/codecs/cs42888.c | 536 + sound/soc/codecs/cs42888.h | 213 6 files changed, 858 insertions(+) create mode 100644 Documentation/devicetree/bindings/sound/cs42888.txt create mode 100644 sound/soc/codecs/cs42888-i2c.c create mode 100644 sound/soc/codecs/cs42888.c create mode 100644 sound/soc/codecs/cs42888.h diff --git a/Documentation/devicetree/bindings/sound/cs42888.txt b/Documentation/devicetree/bindings/sound/cs42888.txt new file mode 100644 index 000..7736527 --- /dev/null +++ b/Documentation/devicetree/bindings/sound/cs42888.txt @@ -0,0 +1,27 @@ +CS42888 audio CODEC + +Required properties: + + - compatible : cirrus,cs42888 + + - reg : the I2C address of the device for I2C + + - clocks : phandle to the clock source for MCLK + + - clock-names : must contain mclk. If you use clock-names, please define the clocks property in terms of clock-names to avoid redundancy. e.g. - clocks: a list of phandles + clock-specifiers, one for each entry in clock-names. It makes the relationship between the two clearer, and makes it far easier to alter the binding in future if required. Cheers, Mark. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] ASoC: cs42888: Add codec driver support
On Tue, Mar 11, 2014 at 11:30:53AM +, Mark Rutland wrote: On Tue, Mar 11, 2014 at 11:19:51AM +, Nicolin Chen wrote: This patch adds support for the Cirrus Logic CS42888 Audio CODEC that has four 24-bit A/D and eight 24-bit D/A converters. [ CS42888 supports both I2C and SPI control ports. As initial patch, this patch only adds the support for I2C. ] Signed-off-by: Nicolin Chen guangyu.c...@freescale.com --- Changelog v2: * Separated driver into I2C and main part drivers. * Added error out if failed to get clock. * Enabled Auto-mute function as default. * Disabled runtime-pm in remove(). * Added return value checking for regcache_sync(). * Fixed some trival parts like 'const' as Lars mentioned. .../devicetree/bindings/sound/cs42888.txt | 27 ++ sound/soc/codecs/Kconfig | 10 + sound/soc/codecs/Makefile | 4 + sound/soc/codecs/cs42888-i2c.c | 68 +++ sound/soc/codecs/cs42888.c | 536 + sound/soc/codecs/cs42888.h | 213 6 files changed, 858 insertions(+) create mode 100644 Documentation/devicetree/bindings/sound/cs42888.txt create mode 100644 sound/soc/codecs/cs42888-i2c.c create mode 100644 sound/soc/codecs/cs42888.c create mode 100644 sound/soc/codecs/cs42888.h diff --git a/Documentation/devicetree/bindings/sound/cs42888.txt b/Documentation/devicetree/bindings/sound/cs42888.txt new file mode 100644 index 000..7736527 --- /dev/null +++ b/Documentation/devicetree/bindings/sound/cs42888.txt @@ -0,0 +1,27 @@ +CS42888 audio CODEC + +Required properties: + + - compatible : cirrus,cs42888 + + - reg : the I2C address of the device for I2C + + - clocks : phandle to the clock source for MCLK + + - clock-names : must contain mclk. If you use clock-names, please define the clocks property in terms of clock-names to avoid redundancy. e.g. - clocks: a list of phandles + clock-specifiers, one for each entry in clock-names. It makes the relationship between the two clearer, and makes it far easier to alter the binding in future if required. Thank you for the quick reply. Will fix it in a minute. Best regards, Nicolin Chen -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] ASoC: cs42888: Add codec driver support
On Tue, 11 Mar 2014, Nicolin Chen wrote: This patch adds support for the Cirrus Logic CS42888 Audio CODEC that has four 24-bit A/D and eight 24-bit D/A converters. [ CS42888 supports both I2C and SPI control ports. As initial patch, this patch only adds the support for I2C. ] Signed-off-by: Nicolin Chen guangyu.c...@freescale.com --- So WRT the CS42888, this is one device in a series of 2 devices that are register compatible with the only difference being that the CS42488 has 2 extra ADC's. Same die and same DeviceID. Would it make more sense to submit the driver with the extra ADC's to make it feature complete for both devices? Wouldn't be odd to have 2 drivers that do the same for 98% of the code. Thoughts? Brian -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] ASoC: cs42888: Add codec driver support
On 03/11/2014 04:41 PM, Brian Austin wrote: On Tue, 11 Mar 2014, Nicolin Chen wrote: This patch adds support for the Cirrus Logic CS42888 Audio CODEC that has four 24-bit A/D and eight 24-bit D/A converters. [ CS42888 supports both I2C and SPI control ports. As initial patch, this patch only adds the support for I2C. ] Signed-off-by: Nicolin Chen guangyu.c...@freescale.com --- So WRT the CS42888, this is one device in a series of 2 devices that are register compatible with the only difference being that the CS42488 has 2 extra ADC's. Same die and same DeviceID. Would it make more sense to submit the driver with the extra ADC's to make it feature complete for both devices? Wouldn't be odd to have 2 drivers that do the same for 98% of the code. This should be in one driver. But support for the second device can always be added in a follow up patch. - Lars -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] ASoC: cs42888: Add codec driver support
On Tue, Mar 11, 2014 at 04:55:58PM +0100, Lars-Peter Clausen wrote: On 03/11/2014 04:41 PM, Brian Austin wrote: So WRT the CS42888, this is one device in a series of 2 devices that are register compatible with the only difference being that the CS42488 has 2 extra ADC's. Same die and same DeviceID. Would it make more sense to submit the driver with the extra ADC's to make it feature complete for both devices? Wouldn't be odd to have 2 drivers that do the same for 98% of the code. This should be in one driver. But support for the second device can always be added in a follow up patch. Yes, we can add support later. It might be helpful to name the driver with some generic name I guess. signature.asc Description: Digital signature
Re: [PATCH v2] ASoC: cs42888: Add codec driver support
On Tue, Mar 11, 2014 at 06:46:06PM +, Mark Brown wrote: On Tue, Mar 11, 2014 at 04:55:58PM +0100, Lars-Peter Clausen wrote: On 03/11/2014 04:41 PM, Brian Austin wrote: So WRT the CS42888, this is one device in a series of 2 devices that are register compatible with the only difference being that the CS42488 has 2 extra ADC's. Same die and same DeviceID. Would it make more sense to submit the driver with the extra ADC's to make it feature complete for both devices? Wouldn't be odd to have 2 drivers that do the same for 98% of the code. This should be in one driver. But support for the second device can always be added in a follow up patch. Yes, we can add support later. It might be helpful to name the driver with some generic name I guess. I didn't notice there's a sister model for CS42888. But good to know. And I think I should try cs42x88.c as the driver name, which makes sense even if we have another sister here. Or Brian may give me a more official one? It would be better if I also put those CS42488's support into the driver but I'll start a long vacation from tomorrow. So if I am not able to do that for CS42488's parts in the short time, I'll send the CS42888 part first by the end of the day and we can add CS42488 later. Thanks all, Nicolin Chen -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] ASoC: cs42888: Add codec driver support
Hi Brian, On Wed, Mar 12, 2014 at 02:05:00AM +, Austin, Brian wrote: On Mar 11, 2014, at 13:46, Mark Brown broo...@kernel.org wrote: On Tue, Mar 11, 2014 at 04:55:58PM +0100, Lars-Peter Clausen wrote: On 03/11/2014 04:41 PM, Brian Austin wrote: So WRT the CS42888, this is one device in a series of 2 devices that are register compatible with the only difference being that the CS42488 has 2 extra ADC's. Same die and same DeviceID. Would it make more sense to submit the driver with the extra ADC's to make it feature complete for both devices? Wouldn't be odd to have 2 drivers that do the same for 98% of the code. This should be in one driver. But support for the second device can always be added in a follow up patch. Yes, we can add support later. It might be helpful to name the driver with some generic name I guess. CS42xx8 would work. We have a CS42448 and the CS42888. The difference is the 2 extra ADC's on the CS42448. I can add those later. I still would like to go over some of the v2 code though before a v3 is sent. Actually I've already sent v3 right after this v2 due to some doc updating. Anyway, I'd love to wait for your comments against the patch before I send v3. Thank you, Nicolin -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] ASoC: cs42888: Add codec driver support
On Mar 11, 2014, at 13:46, Mark Brown broo...@kernel.org wrote: On Tue, Mar 11, 2014 at 04:55:58PM +0100, Lars-Peter Clausen wrote: On 03/11/2014 04:41 PM, Brian Austin wrote: So WRT the CS42888, this is one device in a series of 2 devices that are register compatible with the only difference being that the CS42488 has 2 extra ADC's. Same die and same DeviceID. Would it make more sense to submit the driver with the extra ADC's to make it feature complete for both devices? Wouldn't be odd to have 2 drivers that do the same for 98% of the code. This should be in one driver. But support for the second device can always be added in a follow up patch. Yes, we can add support later. It might be helpful to name the driver with some generic name I guess. CS42xx8 would work. We have a CS42448 and the CS42888. The difference is the 2 extra ADC's on the CS42448. I can add those later. I still would like to go over some of the v2 code though before a v3 is sent. Thanks Brian -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
