Message ID | 20210505140854.15929-2-macroalpha82@gmail.com (mailing list archive) |
---|---|
State | Superseded, archived |
Headers | show |
Series | ASoC: codecs: add rk817 support | expand |
On Wed, 05 May 2021, Chris Morgan wrote: > From: Chris Morgan <macromorgan@hotmail.com> > > Add rk817 codec support cell to rk808 mfd driver. > > Tested-by: Maciej Matuszczyk <maccraft123mc@gmail.com> > Signed-off-by: Chris Morgan <macromorgan@hotmail.com> Nit: These should be chronological. > --- > Changes in v9: > - Add cover letter. > - Remove documentation for interrupt parent per Rob Herring's request. > - Remove unused MODULE_DEVICE_TABLE to fix a bug identified by kernel test > robot. > Changes in v8: > - Added additional documentation for missing properties of #sound-dai-cells, > interrupt-parent, and wakeup-source for mfd documentation. > - Corrected order of elements descriptions in device tree documentation. > - Changed name of "mic-in-differential" to "rockchip,mic-in-differential". > - Changed name of sound card from "rockchip,rk817-codec" to "Analog". > - Removed unused resets and reset-names from the i2s1_2ch node. > Changes in v7: > - Removed ifdef around register definitions for MFD. > - Replaced codec documentation with updates to MFD documentation. > - Reordered elements in example to comply with upstream rules. > - Added binding update back for Odroid Go Advance as requested. > - Submitting patches from gmail now. > Changes in v6: > - Included additional project maintainers for correct subsystems. > - Removed unneeded compatible from DT documentation. > - Removed binding update for Odroid Go Advance (will do in seperate series). > Changes in v5: > - Move register definitions from rk817_codec.h to main rk808.h register > definitions. > - Add volatile register for codec bits. > - Add default values for codec bits. > - Removed of_compatible from mtd driver (not necessary). > - Switched to using parent regmap instead of private regmap for codec. > Changes in v4: > - Created set_pll() call. > - Created user visible gain control in mic. > - Check for return value of clk_prepare_enable(). > - Removed duplicate clk_prepare_enable(). > - Split DT documentation to separate commit. > Changes in v3: > - Use DAPM macros to set audio path. > - Updated devicetree binding (as every rk817 has this codec chip). > - Changed documentation to yaml format. > - Split MFD changes to separate commit. > Changes in v2: > - Fixed audio path registers to solve some bugs. > > drivers/mfd/rk808.c | 85 +++++++++++++++++++++++++++++++++++++++ > include/linux/mfd/rk808.h | 81 +++++++++++++++++++++++++++++++++++++ > 2 files changed, 166 insertions(+) > > diff --git a/drivers/mfd/rk808.c b/drivers/mfd/rk808.c > index ad923dd4e007..9231209184e0 100644 > --- a/drivers/mfd/rk808.c > +++ b/drivers/mfd/rk808.c > @@ -65,6 +65,7 @@ static bool rk817_is_volatile_reg(struct device *dev, unsigned int reg) > switch (reg) { > case RK817_SECONDS_REG ... RK817_WEEKS_REG: > case RK817_RTC_STATUS_REG: > + case RK817_CODEC_DTOP_LPT_SRST: > case RK817_INT_STS_REG0: > case RK817_INT_STS_REG1: > case RK817_INT_STS_REG2: > @@ -163,6 +164,11 @@ static const struct mfd_cell rk817s[] = { > .num_resources = ARRAY_SIZE(rk817_rtc_resources), > .resources = &rk817_rtc_resources[0], > }, > +#ifdef CONFIG_SND_SOC_RK817 > + { > + .name = "rk817-codec", > + }, > +#endif No #ifery please. Just replace it with a comment. If no associated driver exists, it just won't match/bind.
On Mon, May 10, 2021 at 05:23:29PM +0100, Lee Jones wrote: > On Wed, 05 May 2021, Chris Morgan wrote: > > > From: Chris Morgan <macromorgan@hotmail.com> > > > > Add rk817 codec support cell to rk808 mfd driver. > > > > Tested-by: Maciej Matuszczyk <maccraft123mc@gmail.com> > > Signed-off-by: Chris Morgan <macromorgan@hotmail.com> > > Nit: These should be chronological. Acknowledged. I will make sure to do this if a v10 is necessary. > > > --- > > Changes in v9: > > - Add cover letter. > > - Remove documentation for interrupt parent per Rob Herring's request. > > - Remove unused MODULE_DEVICE_TABLE to fix a bug identified by kernel test > > robot. > > Changes in v8: > > - Added additional documentation for missing properties of #sound-dai-cells, > > interrupt-parent, and wakeup-source for mfd documentation. > > - Corrected order of elements descriptions in device tree documentation. > > - Changed name of "mic-in-differential" to "rockchip,mic-in-differential". > > - Changed name of sound card from "rockchip,rk817-codec" to "Analog". > > - Removed unused resets and reset-names from the i2s1_2ch node. > > Changes in v7: > > - Removed ifdef around register definitions for MFD. > > - Replaced codec documentation with updates to MFD documentation. > > - Reordered elements in example to comply with upstream rules. > > - Added binding update back for Odroid Go Advance as requested. > > - Submitting patches from gmail now. > > Changes in v6: > > - Included additional project maintainers for correct subsystems. > > - Removed unneeded compatible from DT documentation. > > - Removed binding update for Odroid Go Advance (will do in seperate series). > > Changes in v5: > > - Move register definitions from rk817_codec.h to main rk808.h register > > definitions. > > - Add volatile register for codec bits. > > - Add default values for codec bits. > > - Removed of_compatible from mtd driver (not necessary). > > - Switched to using parent regmap instead of private regmap for codec. > > Changes in v4: > > - Created set_pll() call. > > - Created user visible gain control in mic. > > - Check for return value of clk_prepare_enable(). > > - Removed duplicate clk_prepare_enable(). > > - Split DT documentation to separate commit. > > Changes in v3: > > - Use DAPM macros to set audio path. > > - Updated devicetree binding (as every rk817 has this codec chip). > > - Changed documentation to yaml format. > > - Split MFD changes to separate commit. > > Changes in v2: > > - Fixed audio path registers to solve some bugs. > > > > drivers/mfd/rk808.c | 85 +++++++++++++++++++++++++++++++++++++++ > > include/linux/mfd/rk808.h | 81 +++++++++++++++++++++++++++++++++++++ > > 2 files changed, 166 insertions(+) > > > > diff --git a/drivers/mfd/rk808.c b/drivers/mfd/rk808.c > > index ad923dd4e007..9231209184e0 100644 > > --- a/drivers/mfd/rk808.c > > +++ b/drivers/mfd/rk808.c > > @@ -65,6 +65,7 @@ static bool rk817_is_volatile_reg(struct device *dev, unsigned int reg) > > switch (reg) { > > case RK817_SECONDS_REG ... RK817_WEEKS_REG: > > case RK817_RTC_STATUS_REG: > > + case RK817_CODEC_DTOP_LPT_SRST: > > case RK817_INT_STS_REG0: > > case RK817_INT_STS_REG1: > > case RK817_INT_STS_REG2: > > @@ -163,6 +164,11 @@ static const struct mfd_cell rk817s[] = { > > .num_resources = ARRAY_SIZE(rk817_rtc_resources), > > .resources = &rk817_rtc_resources[0], > > }, > > +#ifdef CONFIG_SND_SOC_RK817 > > + { > > + .name = "rk817-codec", > > + }, > > +#endif > > No #ifery please. > > Just replace it with a comment. > > If no associated driver exists, it just won't match/bind. I did the "if" here because I noticed that if I have a rk817 and do not utilize the codec I receive a dmesg warning. I put the if here to silence it in the event that someone was using this PMIC but didn't want to use the audio codec. I will make the change if you say so though, but I just want to confirm that it's acceptable to have a warning for all rk817s that do not use the codec about a missing codec. The hardware is always present, I just can't say for certain it will always be used. Thank you. > > -- > Lee Jones [李琼斯] > Senior Technical Lead - Developer Services > Linaro.org │ Open source software for Arm SoCs > Follow Linaro: Facebook | Twitter | Blog
On Thu, 13 May 2021, Chris Morgan wrote: > On Mon, May 10, 2021 at 05:23:29PM +0100, Lee Jones wrote: > > On Wed, 05 May 2021, Chris Morgan wrote: > > > > > From: Chris Morgan <macromorgan@hotmail.com> > > > > > > Add rk817 codec support cell to rk808 mfd driver. > > > > > > Tested-by: Maciej Matuszczyk <maccraft123mc@gmail.com> > > > Signed-off-by: Chris Morgan <macromorgan@hotmail.com> > > > > Nit: These should be chronological. > > Acknowledged. I will make sure to do this if a v10 is necessary. > > > > > > --- > > > Changes in v9: > > > - Add cover letter. > > > - Remove documentation for interrupt parent per Rob Herring's request. > > > - Remove unused MODULE_DEVICE_TABLE to fix a bug identified by kernel test > > > robot. > > > Changes in v8: > > > - Added additional documentation for missing properties of #sound-dai-cells, > > > interrupt-parent, and wakeup-source for mfd documentation. > > > - Corrected order of elements descriptions in device tree documentation. > > > - Changed name of "mic-in-differential" to "rockchip,mic-in-differential". > > > - Changed name of sound card from "rockchip,rk817-codec" to "Analog". > > > - Removed unused resets and reset-names from the i2s1_2ch node. > > > Changes in v7: > > > - Removed ifdef around register definitions for MFD. > > > - Replaced codec documentation with updates to MFD documentation. > > > - Reordered elements in example to comply with upstream rules. > > > - Added binding update back for Odroid Go Advance as requested. > > > - Submitting patches from gmail now. > > > Changes in v6: > > > - Included additional project maintainers for correct subsystems. > > > - Removed unneeded compatible from DT documentation. > > > - Removed binding update for Odroid Go Advance (will do in seperate series). > > > Changes in v5: > > > - Move register definitions from rk817_codec.h to main rk808.h register > > > definitions. > > > - Add volatile register for codec bits. > > > - Add default values for codec bits. > > > - Removed of_compatible from mtd driver (not necessary). > > > - Switched to using parent regmap instead of private regmap for codec. > > > Changes in v4: > > > - Created set_pll() call. > > > - Created user visible gain control in mic. > > > - Check for return value of clk_prepare_enable(). > > > - Removed duplicate clk_prepare_enable(). > > > - Split DT documentation to separate commit. > > > Changes in v3: > > > - Use DAPM macros to set audio path. > > > - Updated devicetree binding (as every rk817 has this codec chip). > > > - Changed documentation to yaml format. > > > - Split MFD changes to separate commit. > > > Changes in v2: > > > - Fixed audio path registers to solve some bugs. > > > > > > drivers/mfd/rk808.c | 85 +++++++++++++++++++++++++++++++++++++++ > > > include/linux/mfd/rk808.h | 81 +++++++++++++++++++++++++++++++++++++ > > > 2 files changed, 166 insertions(+) > > > > > > diff --git a/drivers/mfd/rk808.c b/drivers/mfd/rk808.c > > > index ad923dd4e007..9231209184e0 100644 > > > --- a/drivers/mfd/rk808.c > > > +++ b/drivers/mfd/rk808.c > > > @@ -65,6 +65,7 @@ static bool rk817_is_volatile_reg(struct device *dev, unsigned int reg) > > > switch (reg) { > > > case RK817_SECONDS_REG ... RK817_WEEKS_REG: > > > case RK817_RTC_STATUS_REG: > > > + case RK817_CODEC_DTOP_LPT_SRST: > > > case RK817_INT_STS_REG0: > > > case RK817_INT_STS_REG1: > > > case RK817_INT_STS_REG2: > > > @@ -163,6 +164,11 @@ static const struct mfd_cell rk817s[] = { > > > .num_resources = ARRAY_SIZE(rk817_rtc_resources), > > > .resources = &rk817_rtc_resources[0], > > > }, > > > +#ifdef CONFIG_SND_SOC_RK817 > > > + { > > > + .name = "rk817-codec", > > > + }, > > > +#endif > > > > No #ifery please. > > > > Just replace it with a comment. > > > > If no associated driver exists, it just won't match/bind. > > I did the "if" here because I noticed that if I have a rk817 and do not > utilize the codec I receive a dmesg warning. I put the if here to silence > it in the event that someone was using this PMIC but didn't want to use > the audio codec. I will make the change if you say so though, but I just > want to confirm that it's acceptable to have a warning for all rk817s > that do not use the codec about a missing codec. The hardware is always > present, I just can't say for certain it will always be used. What is the dmesg warning you receive?
On Thu, May 13, 2021 at 09:11:14PM +0100, Lee Jones wrote: > On Thu, 13 May 2021, Chris Morgan wrote: > > > On Mon, May 10, 2021 at 05:23:29PM +0100, Lee Jones wrote: > > > On Wed, 05 May 2021, Chris Morgan wrote: > > > > > > > From: Chris Morgan <macromorgan@hotmail.com> > > > > > > > > Add rk817 codec support cell to rk808 mfd driver. > > > > > > > > Tested-by: Maciej Matuszczyk <maccraft123mc@gmail.com> > > > > Signed-off-by: Chris Morgan <macromorgan@hotmail.com> > > > > > > Nit: These should be chronological. > > > > Acknowledged. I will make sure to do this if a v10 is necessary. > > > > > > > > > --- > > > > Changes in v9: > > > > - Add cover letter. > > > > - Remove documentation for interrupt parent per Rob Herring's request. > > > > - Remove unused MODULE_DEVICE_TABLE to fix a bug identified by kernel test > > > > robot. > > > > Changes in v8: > > > > - Added additional documentation for missing properties of #sound-dai-cells, > > > > interrupt-parent, and wakeup-source for mfd documentation. > > > > - Corrected order of elements descriptions in device tree documentation. > > > > - Changed name of "mic-in-differential" to "rockchip,mic-in-differential". > > > > - Changed name of sound card from "rockchip,rk817-codec" to "Analog". > > > > - Removed unused resets and reset-names from the i2s1_2ch node. > > > > Changes in v7: > > > > - Removed ifdef around register definitions for MFD. > > > > - Replaced codec documentation with updates to MFD documentation. > > > > - Reordered elements in example to comply with upstream rules. > > > > - Added binding update back for Odroid Go Advance as requested. > > > > - Submitting patches from gmail now. > > > > Changes in v6: > > > > - Included additional project maintainers for correct subsystems. > > > > - Removed unneeded compatible from DT documentation. > > > > - Removed binding update for Odroid Go Advance (will do in seperate series). > > > > Changes in v5: > > > > - Move register definitions from rk817_codec.h to main rk808.h register > > > > definitions. > > > > - Add volatile register for codec bits. > > > > - Add default values for codec bits. > > > > - Removed of_compatible from mtd driver (not necessary). > > > > - Switched to using parent regmap instead of private regmap for codec. > > > > Changes in v4: > > > > - Created set_pll() call. > > > > - Created user visible gain control in mic. > > > > - Check for return value of clk_prepare_enable(). > > > > - Removed duplicate clk_prepare_enable(). > > > > - Split DT documentation to separate commit. > > > > Changes in v3: > > > > - Use DAPM macros to set audio path. > > > > - Updated devicetree binding (as every rk817 has this codec chip). > > > > - Changed documentation to yaml format. > > > > - Split MFD changes to separate commit. > > > > Changes in v2: > > > > - Fixed audio path registers to solve some bugs. > > > > > > > > drivers/mfd/rk808.c | 85 +++++++++++++++++++++++++++++++++++++++ > > > > include/linux/mfd/rk808.h | 81 +++++++++++++++++++++++++++++++++++++ > > > > 2 files changed, 166 insertions(+) > > > > > > > > diff --git a/drivers/mfd/rk808.c b/drivers/mfd/rk808.c > > > > index ad923dd4e007..9231209184e0 100644 > > > > --- a/drivers/mfd/rk808.c > > > > +++ b/drivers/mfd/rk808.c > > > > @@ -65,6 +65,7 @@ static bool rk817_is_volatile_reg(struct device *dev, unsigned int reg) > > > > switch (reg) { > > > > case RK817_SECONDS_REG ... RK817_WEEKS_REG: > > > > case RK817_RTC_STATUS_REG: > > > > + case RK817_CODEC_DTOP_LPT_SRST: > > > > case RK817_INT_STS_REG0: > > > > case RK817_INT_STS_REG1: > > > > case RK817_INT_STS_REG2: > > > > @@ -163,6 +164,11 @@ static const struct mfd_cell rk817s[] = { > > > > .num_resources = ARRAY_SIZE(rk817_rtc_resources), > > > > .resources = &rk817_rtc_resources[0], > > > > }, > > > > +#ifdef CONFIG_SND_SOC_RK817 > > > > + { > > > > + .name = "rk817-codec", > > > > + }, > > > > +#endif > > > > > > No #ifery please. > > > > > > Just replace it with a comment. > > > > > > If no associated driver exists, it just won't match/bind. > > > > I did the "if" here because I noticed that if I have a rk817 and do not > > utilize the codec I receive a dmesg warning. I put the if here to silence > > it in the event that someone was using this PMIC but didn't want to use > > the audio codec. I will make the change if you say so though, but I just > > want to confirm that it's acceptable to have a warning for all rk817s > > that do not use the codec about a missing codec. The hardware is always > > present, I just can't say for certain it will always be used. > > What is the dmesg warning you receive? It appears I was confused, I will update the code. No warning is received when I take away the ifdef guard. However, if I build the codec and don't include a devicetree node for it I get the following lines in dmesg: rk817-codec rk817-codec: rk817_codec_parse_dt_property() Can not get child: codec rk817-codec rk817-codec: rk817_platform_probe() parse device tree property error -19 So it looks like this ifdef was meant to "fix" a problem that it doesn't even fix. I'll get rid of it and resubmit. To that end, do you think these messages above are okay, or should we try to fix them in the edge case of a user with an rk817 who doesn't use the codec but still has the codec driver compiled? Thank you. > > -- > Lee Jones [李琼斯] > Senior Technical Lead - Developer Services > Linaro.org │ Open source software for Arm SoCs > Follow Linaro: Facebook | Twitter | Blog
Hi Chris, Am Freitag, 14. Mai 2021, 17:50:08 CEST schrieb Chris Morgan: > On Thu, May 13, 2021 at 09:11:14PM +0100, Lee Jones wrote: > > On Thu, 13 May 2021, Chris Morgan wrote: > > > > > On Mon, May 10, 2021 at 05:23:29PM +0100, Lee Jones wrote: > > > > On Wed, 05 May 2021, Chris Morgan wrote: > > > > > > > > > From: Chris Morgan <macromorgan@hotmail.com> > > > > > > > > > > Add rk817 codec support cell to rk808 mfd driver. > > > > > > > > > > Tested-by: Maciej Matuszczyk <maccraft123mc@gmail.com> > > > > > Signed-off-by: Chris Morgan <macromorgan@hotmail.com> > > > > > > > > Nit: These should be chronological. > > > > > > Acknowledged. I will make sure to do this if a v10 is necessary. > > > > > > > > > > > > --- > > > > > Changes in v9: > > > > > - Add cover letter. > > > > > - Remove documentation for interrupt parent per Rob Herring's request. > > > > > - Remove unused MODULE_DEVICE_TABLE to fix a bug identified by kernel test > > > > > robot. > > > > > Changes in v8: > > > > > - Added additional documentation for missing properties of #sound-dai-cells, > > > > > interrupt-parent, and wakeup-source for mfd documentation. > > > > > - Corrected order of elements descriptions in device tree documentation. > > > > > - Changed name of "mic-in-differential" to "rockchip,mic-in-differential". > > > > > - Changed name of sound card from "rockchip,rk817-codec" to "Analog". > > > > > - Removed unused resets and reset-names from the i2s1_2ch node. > > > > > Changes in v7: > > > > > - Removed ifdef around register definitions for MFD. > > > > > - Replaced codec documentation with updates to MFD documentation. > > > > > - Reordered elements in example to comply with upstream rules. > > > > > - Added binding update back for Odroid Go Advance as requested. > > > > > - Submitting patches from gmail now. > > > > > Changes in v6: > > > > > - Included additional project maintainers for correct subsystems. > > > > > - Removed unneeded compatible from DT documentation. > > > > > - Removed binding update for Odroid Go Advance (will do in seperate series). > > > > > Changes in v5: > > > > > - Move register definitions from rk817_codec.h to main rk808.h register > > > > > definitions. > > > > > - Add volatile register for codec bits. > > > > > - Add default values for codec bits. > > > > > - Removed of_compatible from mtd driver (not necessary). > > > > > - Switched to using parent regmap instead of private regmap for codec. > > > > > Changes in v4: > > > > > - Created set_pll() call. > > > > > - Created user visible gain control in mic. > > > > > - Check for return value of clk_prepare_enable(). > > > > > - Removed duplicate clk_prepare_enable(). > > > > > - Split DT documentation to separate commit. > > > > > Changes in v3: > > > > > - Use DAPM macros to set audio path. > > > > > - Updated devicetree binding (as every rk817 has this codec chip). > > > > > - Changed documentation to yaml format. > > > > > - Split MFD changes to separate commit. > > > > > Changes in v2: > > > > > - Fixed audio path registers to solve some bugs. > > > > > > > > > > drivers/mfd/rk808.c | 85 +++++++++++++++++++++++++++++++++++++++ > > > > > include/linux/mfd/rk808.h | 81 +++++++++++++++++++++++++++++++++++++ > > > > > 2 files changed, 166 insertions(+) > > > > > > > > > > diff --git a/drivers/mfd/rk808.c b/drivers/mfd/rk808.c > > > > > index ad923dd4e007..9231209184e0 100644 > > > > > --- a/drivers/mfd/rk808.c > > > > > +++ b/drivers/mfd/rk808.c > > > > > @@ -65,6 +65,7 @@ static bool rk817_is_volatile_reg(struct device *dev, unsigned int reg) > > > > > switch (reg) { > > > > > case RK817_SECONDS_REG ... RK817_WEEKS_REG: > > > > > case RK817_RTC_STATUS_REG: > > > > > + case RK817_CODEC_DTOP_LPT_SRST: > > > > > case RK817_INT_STS_REG0: > > > > > case RK817_INT_STS_REG1: > > > > > case RK817_INT_STS_REG2: > > > > > @@ -163,6 +164,11 @@ static const struct mfd_cell rk817s[] = { > > > > > .num_resources = ARRAY_SIZE(rk817_rtc_resources), > > > > > .resources = &rk817_rtc_resources[0], > > > > > }, > > > > > +#ifdef CONFIG_SND_SOC_RK817 > > > > > + { > > > > > + .name = "rk817-codec", > > > > > + }, > > > > > +#endif > > > > > > > > No #ifery please. > > > > > > > > Just replace it with a comment. > > > > > > > > If no associated driver exists, it just won't match/bind. > > > > > > I did the "if" here because I noticed that if I have a rk817 and do not > > > utilize the codec I receive a dmesg warning. I put the if here to silence > > > it in the event that someone was using this PMIC but didn't want to use > > > the audio codec. I will make the change if you say so though, but I just > > > want to confirm that it's acceptable to have a warning for all rk817s > > > that do not use the codec about a missing codec. The hardware is always > > > present, I just can't say for certain it will always be used. > > > > What is the dmesg warning you receive? > > It appears I was confused, I will update the code. No warning is > received when I take away the ifdef guard. However, if I build the > codec and don't include a devicetree node for it I get the following > lines in dmesg: > > rk817-codec rk817-codec: rk817_codec_parse_dt_property() Can not get child: codec > rk817-codec rk817-codec: rk817_platform_probe() parse device tree property error -19 > > So it looks like this ifdef was meant to "fix" a problem that it > doesn't even fix. I'll get rid of it and resubmit. To that end, do you > think these messages above are okay, or should we try to fix them in > the edge case of a user with an rk817 who doesn't use the codec but > still has the codec driver compiled? The general case is always having most stuff enabled (as modules) think distro-kernels. So having the codec available but a board not using it should not result in error messages confusing the user ;-) . I don't think the rk817-codec will be the first mfd to stumble upon this, so I guess just looking through others might provide the solution on how to resolve this "silently" ;-) Heiko
On Fri, May 14, 2021 at 06:36:47PM +0200, Heiko Stuebner wrote: > Hi Chris, > > Am Freitag, 14. Mai 2021, 17:50:08 CEST schrieb Chris Morgan: > > On Thu, May 13, 2021 at 09:11:14PM +0100, Lee Jones wrote: > > > On Thu, 13 May 2021, Chris Morgan wrote: > > > > > > > On Mon, May 10, 2021 at 05:23:29PM +0100, Lee Jones wrote: > > > > > On Wed, 05 May 2021, Chris Morgan wrote: > > > > > > > > > > > From: Chris Morgan <macromorgan@hotmail.com> > > > > > > > > > > > > Add rk817 codec support cell to rk808 mfd driver. > > > > > > > > > > > > Tested-by: Maciej Matuszczyk <maccraft123mc@gmail.com> > > > > > > Signed-off-by: Chris Morgan <macromorgan@hotmail.com> > > > > > > > > > > Nit: These should be chronological. > > > > > > > > Acknowledged. I will make sure to do this if a v10 is necessary. > > > > > > > > > > > > > > > --- > > > > > > Changes in v9: > > > > > > - Add cover letter. > > > > > > - Remove documentation for interrupt parent per Rob Herring's request. > > > > > > - Remove unused MODULE_DEVICE_TABLE to fix a bug identified by kernel test > > > > > > robot. > > > > > > Changes in v8: > > > > > > - Added additional documentation for missing properties of #sound-dai-cells, > > > > > > interrupt-parent, and wakeup-source for mfd documentation. > > > > > > - Corrected order of elements descriptions in device tree documentation. > > > > > > - Changed name of "mic-in-differential" to "rockchip,mic-in-differential". > > > > > > - Changed name of sound card from "rockchip,rk817-codec" to "Analog". > > > > > > - Removed unused resets and reset-names from the i2s1_2ch node. > > > > > > Changes in v7: > > > > > > - Removed ifdef around register definitions for MFD. > > > > > > - Replaced codec documentation with updates to MFD documentation. > > > > > > - Reordered elements in example to comply with upstream rules. > > > > > > - Added binding update back for Odroid Go Advance as requested. > > > > > > - Submitting patches from gmail now. > > > > > > Changes in v6: > > > > > > - Included additional project maintainers for correct subsystems. > > > > > > - Removed unneeded compatible from DT documentation. > > > > > > - Removed binding update for Odroid Go Advance (will do in seperate series). > > > > > > Changes in v5: > > > > > > - Move register definitions from rk817_codec.h to main rk808.h register > > > > > > definitions. > > > > > > - Add volatile register for codec bits. > > > > > > - Add default values for codec bits. > > > > > > - Removed of_compatible from mtd driver (not necessary). > > > > > > - Switched to using parent regmap instead of private regmap for codec. > > > > > > Changes in v4: > > > > > > - Created set_pll() call. > > > > > > - Created user visible gain control in mic. > > > > > > - Check for return value of clk_prepare_enable(). > > > > > > - Removed duplicate clk_prepare_enable(). > > > > > > - Split DT documentation to separate commit. > > > > > > Changes in v3: > > > > > > - Use DAPM macros to set audio path. > > > > > > - Updated devicetree binding (as every rk817 has this codec chip). > > > > > > - Changed documentation to yaml format. > > > > > > - Split MFD changes to separate commit. > > > > > > Changes in v2: > > > > > > - Fixed audio path registers to solve some bugs. > > > > > > > > > > > > drivers/mfd/rk808.c | 85 +++++++++++++++++++++++++++++++++++++++ > > > > > > include/linux/mfd/rk808.h | 81 +++++++++++++++++++++++++++++++++++++ > > > > > > 2 files changed, 166 insertions(+) > > > > > > > > > > > > diff --git a/drivers/mfd/rk808.c b/drivers/mfd/rk808.c > > > > > > index ad923dd4e007..9231209184e0 100644 > > > > > > --- a/drivers/mfd/rk808.c > > > > > > +++ b/drivers/mfd/rk808.c > > > > > > @@ -65,6 +65,7 @@ static bool rk817_is_volatile_reg(struct device *dev, unsigned int reg) > > > > > > switch (reg) { > > > > > > case RK817_SECONDS_REG ... RK817_WEEKS_REG: > > > > > > case RK817_RTC_STATUS_REG: > > > > > > + case RK817_CODEC_DTOP_LPT_SRST: > > > > > > case RK817_INT_STS_REG0: > > > > > > case RK817_INT_STS_REG1: > > > > > > case RK817_INT_STS_REG2: > > > > > > @@ -163,6 +164,11 @@ static const struct mfd_cell rk817s[] = { > > > > > > .num_resources = ARRAY_SIZE(rk817_rtc_resources), > > > > > > .resources = &rk817_rtc_resources[0], > > > > > > }, > > > > > > +#ifdef CONFIG_SND_SOC_RK817 > > > > > > + { > > > > > > + .name = "rk817-codec", > > > > > > + }, > > > > > > +#endif > > > > > > > > > > No #ifery please. > > > > > > > > > > Just replace it with a comment. > > > > > > > > > > If no associated driver exists, it just won't match/bind. > > > > > > > > I did the "if" here because I noticed that if I have a rk817 and do not > > > > utilize the codec I receive a dmesg warning. I put the if here to silence > > > > it in the event that someone was using this PMIC but didn't want to use > > > > the audio codec. I will make the change if you say so though, but I just > > > > want to confirm that it's acceptable to have a warning for all rk817s > > > > that do not use the codec about a missing codec. The hardware is always > > > > present, I just can't say for certain it will always be used. > > > > > > What is the dmesg warning you receive? > > > > It appears I was confused, I will update the code. No warning is > > received when I take away the ifdef guard. However, if I build the > > codec and don't include a devicetree node for it I get the following > > lines in dmesg: > > > > rk817-codec rk817-codec: rk817_codec_parse_dt_property() Can not get child: codec > > rk817-codec rk817-codec: rk817_platform_probe() parse device tree property error -19 > > > > So it looks like this ifdef was meant to "fix" a problem that it > > doesn't even fix. I'll get rid of it and resubmit. To that end, do you > > think these messages above are okay, or should we try to fix them in > > the edge case of a user with an rk817 who doesn't use the codec but > > still has the codec driver compiled? > > The general case is always having most stuff enabled (as modules) > think distro-kernels. So having the codec available but a board not > using it should not result in error messages confusing the user ;-) . > > I don't think the rk817-codec will be the first mfd to stumble upon > this, so I guess just looking through others might provide the > solution on how to resolve this "silently" ;-) > > > Heiko > > > > > I'm just going to change the message to a dev_dbg, since it's coming from the codec driver anyway. That way if you purposefully aren't using the codec the messages shouldn't appear in the dmesg log, however if you're troubleshooting a problem that exists because of a missing or invalid entry and enable debug messages you'll see this error. Again, I expect this to be an edge case where a user has an rk817 but purposefully doesn't want to use the codec hardware that's on 100% of all rk817s. Thank you.
diff --git a/drivers/mfd/rk808.c b/drivers/mfd/rk808.c index ad923dd4e007..9231209184e0 100644 --- a/drivers/mfd/rk808.c +++ b/drivers/mfd/rk808.c @@ -65,6 +65,7 @@ static bool rk817_is_volatile_reg(struct device *dev, unsigned int reg) switch (reg) { case RK817_SECONDS_REG ... RK817_WEEKS_REG: case RK817_RTC_STATUS_REG: + case RK817_CODEC_DTOP_LPT_SRST: case RK817_INT_STS_REG0: case RK817_INT_STS_REG1: case RK817_INT_STS_REG2: @@ -163,6 +164,11 @@ static const struct mfd_cell rk817s[] = { .num_resources = ARRAY_SIZE(rk817_rtc_resources), .resources = &rk817_rtc_resources[0], }, +#ifdef CONFIG_SND_SOC_RK817 + { + .name = "rk817-codec", + }, +#endif }; static const struct mfd_cell rk818s[] = { @@ -201,6 +207,85 @@ static const struct rk808_reg_data rk808_pre_init_reg[] = { static const struct rk808_reg_data rk817_pre_init_reg[] = { {RK817_RTC_CTRL_REG, RTC_STOP, RTC_STOP}, + /* Codec specific registers */ + { RK817_CODEC_DTOP_VUCTL, MASK_ALL, 0x03 }, + { RK817_CODEC_DTOP_VUCTIME, MASK_ALL, 0x00 }, + { RK817_CODEC_DTOP_LPT_SRST, MASK_ALL, 0x00 }, + { RK817_CODEC_DTOP_DIGEN_CLKE, MASK_ALL, 0x00 }, + /* from vendor driver, CODEC_AREF_RTCFG0 not defined in data sheet */ + { RK817_CODEC_AREF_RTCFG0, MASK_ALL, 0x00 }, + { RK817_CODEC_AREF_RTCFG1, MASK_ALL, 0x06 }, + { RK817_CODEC_AADC_CFG0, MASK_ALL, 0xc8 }, + /* from vendor driver, CODEC_AADC_CFG1 not defined in data sheet */ + { RK817_CODEC_AADC_CFG1, MASK_ALL, 0x00 }, + { RK817_CODEC_DADC_VOLL, MASK_ALL, 0x00 }, + { RK817_CODEC_DADC_VOLR, MASK_ALL, 0x00 }, + { RK817_CODEC_DADC_SR_ACL0, MASK_ALL, 0x00 }, + { RK817_CODEC_DADC_ALC1, MASK_ALL, 0x00 }, + { RK817_CODEC_DADC_ALC2, MASK_ALL, 0x00 }, + { RK817_CODEC_DADC_NG, MASK_ALL, 0x00 }, + { RK817_CODEC_DADC_HPF, MASK_ALL, 0x00 }, + { RK817_CODEC_DADC_RVOLL, MASK_ALL, 0xff }, + { RK817_CODEC_DADC_RVOLR, MASK_ALL, 0xff }, + { RK817_CODEC_AMIC_CFG0, MASK_ALL, 0x70 }, + { RK817_CODEC_AMIC_CFG1, MASK_ALL, 0x00 }, + { RK817_CODEC_DMIC_PGA_GAIN, MASK_ALL, 0x66 }, + { RK817_CODEC_DMIC_LMT1, MASK_ALL, 0x00 }, + { RK817_CODEC_DMIC_LMT2, MASK_ALL, 0x00 }, + { RK817_CODEC_DMIC_NG1, MASK_ALL, 0x00 }, + { RK817_CODEC_DMIC_NG2, MASK_ALL, 0x00 }, + /* from vendor driver, CODEC_ADAC_CFG0 not defined in data sheet */ + { RK817_CODEC_ADAC_CFG0, MASK_ALL, 0x00 }, + { RK817_CODEC_ADAC_CFG1, MASK_ALL, 0x07 }, + { RK817_CODEC_DDAC_POPD_DACST, MASK_ALL, 0x82 }, + { RK817_CODEC_DDAC_VOLL, MASK_ALL, 0x00 }, + { RK817_CODEC_DDAC_VOLR, MASK_ALL, 0x00 }, + { RK817_CODEC_DDAC_SR_LMT0, MASK_ALL, 0x00 }, + { RK817_CODEC_DDAC_LMT1, MASK_ALL, 0x00 }, + { RK817_CODEC_DDAC_LMT2, MASK_ALL, 0x00 }, + { RK817_CODEC_DDAC_MUTE_MIXCTL, MASK_ALL, 0xa0 }, + { RK817_CODEC_DDAC_RVOLL, MASK_ALL, 0xff }, + { RK817_CODEC_DADC_RVOLR, MASK_ALL, 0xff }, + { RK817_CODEC_AMIC_CFG0, MASK_ALL, 0x70 }, + { RK817_CODEC_AMIC_CFG1, MASK_ALL, 0x00 }, + { RK817_CODEC_DMIC_PGA_GAIN, MASK_ALL, 0x66 }, + { RK817_CODEC_DMIC_LMT1, MASK_ALL, 0x00 }, + { RK817_CODEC_DMIC_LMT2, MASK_ALL, 0x00 }, + { RK817_CODEC_DMIC_NG1, MASK_ALL, 0x00 }, + { RK817_CODEC_DMIC_NG2, MASK_ALL, 0x00 }, + /* from vendor driver, CODEC_ADAC_CFG0 not defined in data sheet */ + { RK817_CODEC_ADAC_CFG0, MASK_ALL, 0x00 }, + { RK817_CODEC_ADAC_CFG1, MASK_ALL, 0x07 }, + { RK817_CODEC_DDAC_POPD_DACST, MASK_ALL, 0x82 }, + { RK817_CODEC_DDAC_VOLL, MASK_ALL, 0x00 }, + { RK817_CODEC_DDAC_VOLR, MASK_ALL, 0x00 }, + { RK817_CODEC_DDAC_SR_LMT0, MASK_ALL, 0x00 }, + { RK817_CODEC_DDAC_LMT1, MASK_ALL, 0x00 }, + { RK817_CODEC_DDAC_LMT2, MASK_ALL, 0x00 }, + { RK817_CODEC_DDAC_MUTE_MIXCTL, MASK_ALL, 0xa0 }, + { RK817_CODEC_DDAC_RVOLL, MASK_ALL, 0xff }, + { RK817_CODEC_DDAC_RVOLR, MASK_ALL, 0xff }, + { RK817_CODEC_AHP_ANTI0, MASK_ALL, 0x00 }, + { RK817_CODEC_AHP_ANTI1, MASK_ALL, 0x00 }, + { RK817_CODEC_AHP_CFG0, MASK_ALL, 0xe0 }, + { RK817_CODEC_AHP_CFG1, MASK_ALL, 0x1f }, + { RK817_CODEC_AHP_CP, MASK_ALL, 0x09 }, + { RK817_CODEC_ACLASSD_CFG1, MASK_ALL, 0x69 }, + { RK817_CODEC_ACLASSD_CFG2, MASK_ALL, 0x44 }, + { RK817_CODEC_APLL_CFG0, MASK_ALL, 0x04 }, + { RK817_CODEC_APLL_CFG1, MASK_ALL, 0x00 }, + { RK817_CODEC_APLL_CFG2, MASK_ALL, 0x30 }, + { RK817_CODEC_APLL_CFG3, MASK_ALL, 0x19 }, + { RK817_CODEC_APLL_CFG4, MASK_ALL, 0x65 }, + { RK817_CODEC_APLL_CFG5, MASK_ALL, 0x01 }, + { RK817_CODEC_DI2S_CKM, MASK_ALL, 0x01 }, + { RK817_CODEC_DI2S_RSD, MASK_ALL, 0x00 }, + { RK817_CODEC_DI2S_RXCR1, MASK_ALL, 0x00 }, + { RK817_CODEC_DI2S_RXCR2, MASK_ALL, 0x17 }, + { RK817_CODEC_DI2S_RXCMD_TSD, MASK_ALL, 0x00 }, + { RK817_CODEC_DI2S_TXCR1, MASK_ALL, 0x00 }, + { RK817_CODEC_DI2S_TXCR2, MASK_ALL, 0x17 }, + { RK817_CODEC_DI2S_TXCR3_TXCMD, MASK_ALL, 0x00 }, {RK817_GPIO_INT_CFG, RK817_INT_POL_MSK, RK817_INT_POL_L}, {RK817_SYS_CFG(1), RK817_HOTDIE_TEMP_MSK | RK817_TSD_TEMP_MSK, RK817_HOTDIE_105 | RK817_TSD_140}, diff --git a/include/linux/mfd/rk808.h b/include/linux/mfd/rk808.h index e07f6e61cd38..a96e6d43ca06 100644 --- a/include/linux/mfd/rk808.h +++ b/include/linux/mfd/rk808.h @@ -437,6 +437,87 @@ enum rk809_reg_id { #define RK817_RTC_COMP_LSB_REG 0x10 #define RK817_RTC_COMP_MSB_REG 0x11 +/* RK817 Codec Registers */ +#define RK817_CODEC_DTOP_VUCTL 0x12 +#define RK817_CODEC_DTOP_VUCTIME 0x13 +#define RK817_CODEC_DTOP_LPT_SRST 0x14 +#define RK817_CODEC_DTOP_DIGEN_CLKE 0x15 +#define RK817_CODEC_AREF_RTCFG0 0x16 +#define RK817_CODEC_AREF_RTCFG1 0x17 +#define RK817_CODEC_AADC_CFG0 0x18 +#define RK817_CODEC_AADC_CFG1 0x19 +#define RK817_CODEC_DADC_VOLL 0x1a +#define RK817_CODEC_DADC_VOLR 0x1b +#define RK817_CODEC_DADC_SR_ACL0 0x1e +#define RK817_CODEC_DADC_ALC1 0x1f +#define RK817_CODEC_DADC_ALC2 0x20 +#define RK817_CODEC_DADC_NG 0x21 +#define RK817_CODEC_DADC_HPF 0x22 +#define RK817_CODEC_DADC_RVOLL 0x23 +#define RK817_CODEC_DADC_RVOLR 0x24 +#define RK817_CODEC_AMIC_CFG0 0x27 +#define RK817_CODEC_AMIC_CFG1 0x28 +#define RK817_CODEC_DMIC_PGA_GAIN 0x29 +#define RK817_CODEC_DMIC_LMT1 0x2a +#define RK817_CODEC_DMIC_LMT2 0x2b +#define RK817_CODEC_DMIC_NG1 0x2c +#define RK817_CODEC_DMIC_NG2 0x2d +#define RK817_CODEC_ADAC_CFG0 0x2e +#define RK817_CODEC_ADAC_CFG1 0x2f +#define RK817_CODEC_DDAC_POPD_DACST 0x30 +#define RK817_CODEC_DDAC_VOLL 0x31 +#define RK817_CODEC_DDAC_VOLR 0x32 +#define RK817_CODEC_DDAC_SR_LMT0 0x35 +#define RK817_CODEC_DDAC_LMT1 0x36 +#define RK817_CODEC_DDAC_LMT2 0x37 +#define RK817_CODEC_DDAC_MUTE_MIXCTL 0x38 +#define RK817_CODEC_DDAC_RVOLL 0x39 +#define RK817_CODEC_DDAC_RVOLR 0x3a +#define RK817_CODEC_AHP_ANTI0 0x3b +#define RK817_CODEC_AHP_ANTI1 0x3c +#define RK817_CODEC_AHP_CFG0 0x3d +#define RK817_CODEC_AHP_CFG1 0x3e +#define RK817_CODEC_AHP_CP 0x3f +#define RK817_CODEC_ACLASSD_CFG1 0x40 +#define RK817_CODEC_ACLASSD_CFG2 0x41 +#define RK817_CODEC_APLL_CFG0 0x42 +#define RK817_CODEC_APLL_CFG1 0x43 +#define RK817_CODEC_APLL_CFG2 0x44 +#define RK817_CODEC_APLL_CFG3 0x45 +#define RK817_CODEC_APLL_CFG4 0x46 +#define RK817_CODEC_APLL_CFG5 0x47 +#define RK817_CODEC_DI2S_CKM 0x48 +#define RK817_CODEC_DI2S_RSD 0x49 +#define RK817_CODEC_DI2S_RXCR1 0x4a +#define RK817_CODEC_DI2S_RXCR2 0x4b +#define RK817_CODEC_DI2S_RXCMD_TSD 0x4c +#define RK817_CODEC_DI2S_TXCR1 0x4d +#define RK817_CODEC_DI2S_TXCR2 0x4e +#define RK817_CODEC_DI2S_TXCR3_TXCMD 0x4f + +/* RK817_CODEC_DI2S_CKM */ +#define RK817_I2S_MODE_MASK (0x1 << 0) +#define RK817_I2S_MODE_MST (0x1 << 0) +#define RK817_I2S_MODE_SLV (0x0 << 0) + +/* RK817_CODEC_DDAC_MUTE_MIXCTL */ +#define DACMT_MASK (0x1 << 0) +#define DACMT_ENABLE (0x1 << 0) +#define DACMT_DISABLE (0x0 << 0) + +/* RK817_CODEC_DI2S_RXCR2 */ +#define VDW_RX_24BITS (0x17) +#define VDW_RX_16BITS (0x0f) + +/* RK817_CODEC_DI2S_TXCR2 */ +#define VDW_TX_24BITS (0x17) +#define VDW_TX_16BITS (0x0f) + +/* RK817_CODEC_AMIC_CFG0 */ +#define MIC_DIFF_MASK (0x1 << 7) +#define MIC_DIFF_DIS (0x0 << 7) +#define MIC_DIFF_EN (0x1 << 7) + #define RK817_POWER_EN_REG(i) (0xb1 + (i)) #define RK817_POWER_SLP_EN_REG(i) (0xb5 + (i))