Re: [PATCH 6/7] mfd: wm831x: Correct kerneldoc

[Charles Keepax]

On Mon, Apr 19, 2021 at 10:17:25AM +0200, Krzysztof Kozlowski wrote:
> Correct kerneldoc function name to fix W=1 warning:
> 
>   drivers/mfd/wm831x-core.c:121: warning:
> expecting prototype for wm831x_reg_unlock(). Prototype was for 
> wm831x_reg_lock() instead
> 
> Signed-off-by: Krzysztof Kozlowski 
> ---

Acked-by: Charles Keepax 

Thanks,
Charles

Re: [PATCH] mfd: arizona: Fix rumtime PM imbalance on error

[Charles Keepax]

On Wed, Apr 07, 2021 at 01:11:49PM +0800, Dinghao Liu wrote:
> pm_runtime_get_sync() will increase the rumtime PM counter
> even it returns an error. Thus a pairing decrement is needed
> to prevent refcount leak. Fix this by replacing this API with
> pm_runtime_resume_and_get(), which will not change the runtime
> PM counter on error.
> 
> Signed-off-by: Dinghao Liu 
> ---

Acked-by: Charles Keepax 

Wow, did not realise that was how that worked. Thanks for fixing
up.

Thanks,
Charles

Re: [PATCH 32/32] pinctrl: update pin-control.rst references

[Charles Keepax]

On Thu, Apr 01, 2021 at 02:17:52PM +0200, Mauro Carvalho Chehab wrote:
> Changeset 5513b411ea5b ("Documentation: rename pinctl to pin-control")
> renamed: Documentation/driver-api/pinctl.rst
> to: Documentation/driver-api/pin-control.rst.
> 
> Update the cross-references accordingly.
> 
> Fixes: 5513b411ea5b ("Documentation: rename pinctl to pin-control")
> Signed-off-by: Mauro Carvalho Chehab 
> ---

Acked-by: Charles Keepax 

Thanks,
Charles

Re: [PATCH] ASoC: wm8960: Fix wrong bclk and lrclk with pll enabled for some chips

[Charles Keepax]

On Fri, Mar 19, 2021 at 06:48:46PM +0800, Shengjiu Wang wrote:
> The input MCLK is 12.288MHz, the desired output sysclk is 11.2896MHz
> and sample rate is 44100Hz, with the configuration pllprescale=2,
> postscale=sysclkdiv=1, some chip may have wrong bclk
> and lrclk output with pll enabled in master mode, but with the
> configuration pllprescale=1, postscale=2, the output clock is correct.
> 
> >From Datasheet, the PLL performs best when f2 is between
> 90MHz and 100MHz when the desired sysclk output is 11.2896MHz
> or 12.288MHz, so sysclkdiv = 2 (f2/8) is the best choice.
> 
> So search available sysclk_divs from 2 to 1 other than from 1 to 2.
> 
> Fixes: 84fdc00d519f ("ASoC: codec: wm9860: Refactor PLL out freq search")
> Signed-off-by: Shengjiu Wang 
> ---

Acked-by: Charles Keepax 

Thanks,
Charles

Re: [PATCH v2] ASoC: wm8962: Relax bit clock divider searching

[Charles Keepax]

On Mon, Mar 08, 2021 at 10:34:37AM +0800, Shengjiu Wang wrote:
> With S20_3LE format case, the sysclk = rate * 384,
> the bclk = rate * 20 * 2, there is no proper bclk divider
> for 384 / 40, because current condition needs exact match.
> So driver fails to configure the clocking:
> 
> wm8962 3-001a: Unsupported BCLK ratio 9
> 
> Fix this by relaxing bitclk divider searching, so that when
> no exact value can be derived from sysclk pick the closest
> value greater than expected bitclk.
> 
> Signed-off-by: Shengjiu Wang 
> Reviewed-by: Daniel Baluta 
> ---

Acked-by: Charles Keepax 

Thanks,
Charles

Re: [PATCH] sound: soc: codecs: Fix a spello in the file wm8955.c

[Charles Keepax]

On Sat, Mar 06, 2021 at 05:21:51PM +0530, Bhaskar Chowdhury wrote:
> 
> s/sortd/sorted/
> 
> Signed-off-by: Bhaskar Chowdhury 
> ---
>  sound/soc/codecs/wm8955.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/sound/soc/codecs/wm8955.c b/sound/soc/codecs/wm8955.c
> index 513df47bd87d..538bb8b0db39 100644
> --- a/sound/soc/codecs/wm8955.c
> +++ b/sound/soc/codecs/wm8955.c
> @@ -151,7 +151,7 @@ static int wm8955_pll_factors(struct device *dev,
>   /* The oscilator should run at should be 90-100MHz, and
>* there's a divide by 4 plus an optional divide by 2 in the
>* output path to generate the system clock.  The clock table
> -  * is sortd so we should always generate a suitable target. */
> +  * is sorted so we should always generate a suitable target. */
>   target = Fout * 4;
>   if (target < 9000) {
>   pll->outdiv = 1;
> --
> 2.26.2
> 

Content of the patch looks good, the commit message might need a
little work. The subject line needs to be appropriate for the
subsystem. Would be better to start with "ASoC: wm8955: " and
typo is a more standard term than spello :-)

Thanks,
Charles

Re: [PATCH] ASoC: wm8962: Relax bit clock divider searching

[Charles Keepax]

On Wed, Mar 03, 2021 at 07:21:28PM +0800, Shengjiu Wang wrote:
> With S20_3LE format case, the sysclk = rate * 384,
> the bclk = rate * 20 * 2, there is no proper bclk divider
> for 384 / 40, because current condition needs exact match.
> So driver fails to configure the clocking:
> 
> wm8962 3-001a: Unsupported BCLK ratio 9
> 
> Fix this by relaxing bitclk divider searching, so that when
> no exact value can be derived from sysclk pick the closest
> value greater than expected bitclk.
> 
> Signed-off-by: Shengjiu Wang 
> ---
>  sound/soc/codecs/wm8962.c | 21 +++--
>  1 file changed, 11 insertions(+), 10 deletions(-)
> 
> diff --git a/sound/soc/codecs/wm8962.c b/sound/soc/codecs/wm8962.c
> index ce4666a74793..f5cd22450190 100644
> --- a/sound/soc/codecs/wm8962.c
> +++ b/sound/soc/codecs/wm8962.c
> @@ -2403,6 +2403,7 @@ static const int sysclk_rates[] = {
>  static void wm8962_configure_bclk(struct snd_soc_component *component)
>  {
>   struct wm8962_priv *wm8962 = snd_soc_component_get_drvdata(component);
> + int best, min_diff, diff;
>   int dspclk, i;
>   int clocking2 = 0;
>   int clocking4 = 0;
> @@ -2473,23 +2474,23 @@ static void wm8962_configure_bclk(struct 
> snd_soc_component *component)
>  
>   dev_dbg(component->dev, "DSPCLK is %dHz, BCLK %d\n", dspclk, 
> wm8962->bclk);
>  

Very minor nit but it would probably be nice to have some
equivalent debug statement that prints out the actual BCLK we end
up with. There are a couple of statements printing the requested
speed, but nothing that will output what the driver actually
applies after this change.

Otherwise I think the change looks good.

Thanks,
Charles

Re: [PATCH v2] ASoC: wm8960: Remove bitclk relax condition in wm8960_configure_sysclk

[Charles Keepax]

On Wed, Mar 03, 2021 at 11:07:42AM +0800, Shengjiu Wang wrote:
> The call sequence in wm8960_configure_clocking is
> 
>ret = wm8960_configure_sysclk();
>if (ret >= 0)
> goto configure_clock;
> 
>
> 
>ret = wm8960_configure_pll();
> 
> configure_clock:
>...
> 
> wm8960_configure_sysclk is called before wm8960_configure_pll, as
> there is bitclk relax on both functions, so wm8960_configure_sysclk
> always return success, then wm8960_configure_pll() never be called.
> 
> With this case:
> aplay -Dhw:0,0 -d 5 -r 48000 -f S24_LE -c 2 audio48k24b2c.wav
> the required bitclk is 48000 * 24 * 2 = 2304000, bitclk got from
> wm8960_configure_sysclk is 3072000, but if go to wm8960_configure_pll.
> it can get correct bitclk 2304000.
> 
> So bitclk relax condition should be removed in wm8960_configure_sysclk,
> then wm8960_configure_pll can be called, and there is also bitclk relax
> function in wm8960_configure_pll.
> 
> Fixes: 3c01b9ee2ab9 ("ASoC: codec: wm8960: Relax bit clock computation")
> Signed-off-by: Shengjiu Wang 
> Signed-off-by: Daniel Baluta 
> ---

Acked-by: Charles Keepax 

Thanks,
Charles

Re: [PATCH] ASoC: wm8960: Remove bitclk relax condition

[Charles Keepax]

On Tue, Mar 02, 2021 at 07:18:11PM +0800, Shengjiu Wang wrote:
> From: Daniel Baluta 
> 
> Using a higher bitclk then expected doesn't always work.
> Here is an example:
> 
> aplay -Dhw:0,0 -d 5 -r 48000 -f S24_LE -c 2 audio48k24b2c.wav
> 
> In this case, the required bitclk is 48000 * 24 * 2 = 2304000
> but the closest bitclk that can be derived is 3072000. Since
> the clock is faster than expected, it will start to send bytes
> from the next channel so the sound will be corrupted.
> 
> Fixes: 82bab88910ee ("ASoC: codec: wm8960: Relax bit clock computation when 
> using PLL")
> Fixes: 3c01b9ee2ab9 ("ASoC: codec: wm8960: Relax bit clock computation")
> Signed-off-by: Daniel Baluta 
> Signed-off-by: Shengjiu Wang 
> ---

I think this is probably going to need a much more involved fix.
The problem is that there are systems that depend on this
behaviour, so you can't just flat out revert it. And to be fair
the I2S specification says that bit clock can run at a higher
rate than required for the data, so the behaviour is correct as
well.

Probably the best solution here is to add additional contraints
from the machine driver on which rates/bit depths/channels are
supported.

Thanks,
Charles

Antworte zurück für die sofortige Inanspruchnahme deiner gespendeten Gelder und Glückwünsche an dich

[Charles Jackson Jr]

-- 
IHR E-MAIL-KONTO WURDE FÜR EINE SPENDE VON 3.500.000,00 USD FÜR
CHARITY AUSGEWÄHLT. Antworten Sie auf die folgende E-Mail, um weitere
Informationen zu erhalten

E-Mail: charlesjackson...@gmail.com

[PATCH] ASoC: wm_adsp: Remove unused control callback structure

[Charles Keepax]

This callback structure has never been used and it is not clear why it
was added in the first place. Remove it to clear up the code a little.

Signed-off-by: Charles Keepax 
---
 sound/soc/codecs/wm_adsp.c | 10 --
 1 file changed, 10 deletions(-)

diff --git a/sound/soc/codecs/wm_adsp.c b/sound/soc/codecs/wm_adsp.c
index 262c2db4f81ce..070ca7d8c6610 100644
--- a/sound/soc/codecs/wm_adsp.c
+++ b/sound/soc/codecs/wm_adsp.c
@@ -595,13 +595,6 @@ static const struct {
[WM_ADSP_FW_MISC] = { .file = "misc" },
 };
 
-struct wm_coeff_ctl_ops {
-   int (*xget)(struct snd_kcontrol *kcontrol,
-   struct snd_ctl_elem_value *ucontrol);
-   int (*xput)(struct snd_kcontrol *kcontrol,
-   struct snd_ctl_elem_value *ucontrol);
-};
-
 struct wm_coeff_ctl {
const char *name;
const char *fw_name;
@@ -609,7 +602,6 @@ struct wm_coeff_ctl {
const char *subname;
unsigned int subname_len;
struct wm_adsp_alg_region alg_region;
-   struct wm_coeff_ctl_ops ops;
struct wm_adsp *dsp;
unsigned int enabled:1;
struct list_head list;
@@ -1497,8 +1489,6 @@ static int wm_adsp_create_control(struct wm_adsp *dsp,
}
ctl->enabled = 1;
ctl->set = 0;
-   ctl->ops.xget = wm_coeff_get;
-   ctl->ops.xput = wm_coeff_put;
ctl->dsp = dsp;
 
ctl->flags = flags;
-- 
2.11.0

Re: [PATCH -next] mfd: arizona: Make some symbols static

[Charles Keepax]

On Wed, Feb 10, 2021 at 07:56:26AM +, Wei Yongjun wrote:
> The sparse tool complains as follows:
> 
> drivers/mfd/arizona-spi.c:28:31: warning:
>  symbol 'reset_gpios' was not declared. Should it be static?
> drivers/mfd/arizona-spi.c:29:31: warning:
>  symbol 'ldoena_gpios' was not declared. Should it be static?
> 
> Those symbols are not used outside of arizona-spi.c, so this
> commit marks them static.
> 
> Fixes: e933836744a2 ("mfd: arizona: Add support for ACPI enumeration of 
> WM5102 connected over SPI")
> Reported-by: Hulk Robot 
> Signed-off-by: Wei Yongjun 
> ---

Acked-by: Charles Keepax 

Thanks,
Charles

Antworte zurück für den sofortigen Anspruch auf dein gespendetes Geld und gratuliere dir ......

[Charles Jackson Jr]

-- 
IHR E-MAIL-KONTO WURDE FÜR EINE SPENDE VON 3.500.000,00 USD FÜR
CHARITY AUSGEWÄHLT. Antworten Sie auf die folgende E-Mail, um weitere
Informationen zu erhalten

E-Mail: charlesjackson...@gmail.com

Re: [PATCH v4 11/13] ASoC: arizona-jack: Cleanup logging

[Charles Keepax]

On Sat, Jan 23, 2021 at 01:17:18PM +0100, Hans de Goede wrote:
> Cleanup the use of dev_foo functions used for logging:
> 
> 1. Many of these are unnecessarily split over multiple lines
> 2. Use dev_err_probe() in cases where we might get a -EPROBE_DEFER
>return value
> 
> Suggested-by: Andy Shevchenko 
> Signed-off-by: Hans de Goede 
> ---

Acked-by: Charles Keepax 
Tested-by: Charles Keepax 

Thanks,
Charles

Re: [PATCH v4 01/13] mfd: arizona: Drop arizona-extcon cells

[Charles Keepax]

On Sat, Jan 23, 2021 at 01:13:01PM +0100, Hans de Goede wrote:
> The arizona jack-dection handling is being reworked so that the
> codec-child-device drivers directly handle jack-detect themselves,
> so it is no longer necessary to instantiate "arizona-extcon"
> child-devices.
> 
> Signed-off-by: Hans de Goede 
> ---

Acked-by: Charles Keepax 
Tested-by: Charles Keepax 

Thanks,
Charles

Re: [PATCH v4 09/13] ASoC: arizona-jack: convert into a helper library for codec drivers

[Charles Keepax]

On Sat, Jan 23, 2021 at 01:13:09PM +0100, Hans de Goede wrote:
> Convert the arizona extcon driver into a helper library for direct use
> from the arizona codec-drivers, rather then being bound to a separate
> MFD cell.
> 
> Note the probe (and remove) sequence is split into 2 parts:
> 
> 1. The arizona_jack_codec_dev_probe() function inits a bunch of
> jack-detect specific variables in struct arizona_priv and tries to get
> a number of resources where getting them may fail with -EPROBE_DEFER.
> 
> 2. Then once the machine driver has create a snd_sock_jack through
> snd_soc_card_jack_new() it calls snd_soc_component_set_jack() on
> the codec component, which will call the new arizona_jack_set_jack(),
> which sets up jack-detection and requests the IRQs.
> 
> This split is necessary, because the IRQ handlers need access to the
> arizona->dapm pointer and the snd_sock_jack which are not available
> when the codec-driver's probe function runs.
> 
> Note this requires that machine-drivers for codecs which are converted
> to use the new helper functions from arizona-jack.c are modified to
> create a snd_soc_jack through snd_soc_card_jack_new() and register
> this jack with the codec through snd_soc_component_set_jack().
> 
> Reviewed-by: Andy Shevchenko 
> Signed-off-by: Hans de Goede 
> ---

Acked-by: Charles Keepax 
Tested-by: Charles Keepax 

Thanks,
Charles

Re: [PATCH v4 02/13] extcon: arizona: Fix some issues when HPDET IRQ fires after the jack has been unplugged

[Charles Keepax]

On Sat, Jan 23, 2021 at 01:13:02PM +0100, Hans de Goede wrote:
> When the jack is partially inserted and then removed again it may be
> removed while the hpdet code is running. In this case the following
> may happen:
> 
> 1. The "JACKDET rise" or ""JACKDET fall" IRQ triggers
> 2. arizona_jackdet runs and takes info->lock
> 3. The "HPDET" IRQ triggers
> 4. arizona_hpdet_irq runs, blocks on info->lock
> 5. arizona_jackdet calls arizona_stop_mic() and clears info->hpdet_done
> 6. arizona_jackdet releases info->lock
> 7. arizona_hpdet_irq now can continue running and:
> 7.1 Calls arizona_start_mic() (if a mic was detected)
> 7.2 sets info->hpdet_done
> 
> Step 7 is undesirable / a bug:
> 7.1 causes the device to stay in a high power-state (with MICVDD enabled)
> 7.2 causes hpdet to not run on the next jack insertion, which in turn
> causes the EXTCON_JACK_HEADPHONE state to never get set
> 
> This fixes both issues by skipping these 2 steps when arizona_hpdet_irq
> runs after the jack has been unplugged.
> 
> Reviewed-by: Andy Shevchenko 
> Acked-by: Charles Keepax 
> Signed-off-by: Hans de Goede 
> ---

Tested-by: Charles Keepax 

Thanks,
Charles

Re: [PATCH v4 08/13] ASoC: arizona-jack: Use arizona->dev for runtime-pm

[Charles Keepax]

On Sat, Jan 23, 2021 at 01:13:08PM +0100, Hans de Goede wrote:
> Drivers for MFD child-devices such as the arizona codec drivers
> and the arizona-extcon driver can choose to either make
> runtime_pm_get/_put calls on their own child-device, which will
> then be propagated to their parent; or they can make them directly
> on their MFD parent-device.
> 
> The arizona-extcon code was using runtime_pm_get/_put calls on
> its own child-device where as the codec drivers are using
> runtime_pm_get/_put calls on their parent.
> 
> The arizona-extcon MFD cell/child-device has been removed and this
> commit is part of refactoring the arizona-extcon code into a library
> to be used directly from the codec drivers.
> 
> Specifically this commit moves the code over to make
> runtime_pm_get/_put calls on the parent device (on arizona->dev)
> bringing the code inline with how the codec drivers do this.
> 
> Note this also removes the pm_runtime_enable/_disable calls
> as pm_runtime support has already been enabled on the parent-device
> by the arizona MFD driver.
> 
> This is part of a patch series converting the arizona extcon driver into
> a helper library for letting the arizona codec-drivers directly report
> jack state through the standard sound/soc/soc-jack.c functions.
> 
> Reviewed-by: Andy Shevchenko 
> Signed-off-by: Hans de Goede 
> ---

Acked-by: Charles Keepax 
Tested-by: Charles Keepax 

Thanks,
Charles

Re: [PATCH v4 07/13] ASoC: arizona-jack: Move jack-detect variables to struct arizona_priv

[Charles Keepax]

On Sat, Jan 23, 2021 at 01:13:07PM +0100, Hans de Goede wrote:
> Move all the jack-detect variables from struct arizona_extcon_info to
> struct arizona_priv.
> 
> This is part of a patch series converting the arizona extcon driver into
> a helper library for letting the arizona codec-drivers directly report jack
> state through the standard sound/soc/soc-jack.c functions.
> 
> Reviewed-by: Andy Shevchenko 
> Acked-by: Charles Keepax 
> Signed-off-by: Hans de Goede 
> ---

Tested-by: Charles Keepax 

Thanks,
Charles

Re: [PATCH v4 12/13] ASoC: arizona: Make the wm5102, wm5110, wm8997 and wm8998 drivers use the new jack library

[Charles Keepax]

On Sat, Jan 23, 2021 at 01:17:19PM +0100, Hans de Goede wrote:
> Make all arizona codec drivers for which drivers/mfd/arizona-core.c used
> to instantiate a "arizona-extcon" child-device use the new arizona-jack.c
> library for jack-detection.
> 
> This has been tested on a Lenovo Yoga Tablet 2 1051L with a WM5102 codec.
> 
> Reviewed-by: Andy Shevchenko 
> Signed-off-by: Hans de Goede 
> ---

Acked-by: Charles Keepax 
Tested-by: Charles Keepax 

Thanks,
Charles

Re: [PATCH v4 10/13] ASoC: arizona-jack: Use snd_soc_jack to report jack events

[Charles Keepax]

On Sat, Jan 23, 2021 at 01:17:17PM +0100, Hans de Goede wrote:
> Use the snd_soc_jack code to report jack events, instead of using extcon
> for reporting the cable-type + an input_dev for reporting the button
> presses.
> 
> The snd_soc_jack code will report the cable-type through both input_dev
> events and through ALSA controls and the button-presses through input_dev
> events.
> 
> Note that this means that when the codec drivers are moved over to use
> the new arizona-jack.c library code instead of having a separate MFD
> extcon cell with the extcon-arizona.c driver, we will no longer report
> extcon events to userspace for cable-type changes. This should not be
> a problem since "standard" Linux distro userspace does not (and has
> never) used the extcon class interface for this. Android does have
> support for the extcon class interface, but that was introduced in
> the same release as support for input_dev cable-type events, so this
> should not be a problem for Android either.
> 
> Note this also reduces ARIZONA_MAX_MICD_RANGE from 8 to 6, this is
> ok to do since this info is always provided through pdata (or defaults)
> and cannot be overridden from devicetree. All in-kernel users of the
> pdata (and the fallback defaults) define 6 or less buttons/ranges.
> 
> Reviewed-by: Andy Shevchenko 
> Signed-off-by: Hans de Goede 
> ---

Acked-by: Charles Keepax 
Tested-by: Charles Keepax 

Thanks,
Charles

Re: [PATCH v4 13/13] ASoC: Intel: bytcr_wm5102: Add jack detect support

[Charles Keepax]

On Sat, Jan 23, 2021 at 01:17:20PM +0100, Hans de Goede wrote:
> Add jack detect support by creating a jack and calling
> snd_soc_component_set_jack to register the created jack
> with the codec.
> 
> Reviewed-by: Andy Shevchenko 
> Signed-off-by: Hans de Goede 
> ---
> +static struct snd_soc_jack_pin byt_wm5102_pins[] = {
> + {
> + .pin= "Headphone",
> + .mask   = SND_JACK_HEADPHONE,
> + },
> + {
> + .pin= "Headset Mic",
> + .mask   = SND_JACK_MICROPHONE,
> + },
> +};
> +

This patch looks fine to me, but I did have one small question.
What is the thinking behind punting this to the machine driver?

I guess you can not register it if there is no jack present
on the board, or if you have multiple jacks name them
meaningfully. Although I sort of feel like those applied to
the old extcon approach that just internally registered all
the interfaces.

But to be clear not asking for any changes just more about trying
to refine my understanding of things.

Thanks,
Charles

Re: [PATCH v4 03/13] extcon: arizona: Fix various races on driver unbind

[Charles Keepax]

On Sat, Jan 23, 2021 at 01:13:03PM +0100, Hans de Goede wrote:
> We must free/disable all interrupts and cancel all pending works
> before doing further cleanup.
> 
> Before this commit arizona_extcon_remove() was doing several
> register writes to shut things down before disabling the IRQs
> and it was cancelling only 1 of the 3 different works used.
> 
> Move all the register-writes shutting things down to after
> the disabling of the IRQs and add the 2 missing
> cancel_delayed_work_sync() calls.
> 
> This fixes various possible races on driver unbind. One of which
> would always trigger on devices using the mic-clamp feature for
> jack detection. The ARIZONA_MICD_CLAMP_MODE_MASK update was
> done before disabling the IRQs, causing:
> 1. arizona_jackdet() to run
> 2. detect a jack being inserted (clamp disabled means jack inserted)
> 3. call arizona_start_mic() which:
> 3.1 Enables the MICVDD regulator
> 3.2 takes a pm_runtime_reference
> 
> And this was all happening after the ARIZONA_MICD_ENA bit clearing,
> which would undo 3.1 and 3.2 because the ARIZONA_MICD_CLAMP_MODE_MASK
> update was being done after the ARIZONA_MICD_ENA bit clearing.
> 
> So this means that arizona_extcon_remove() would exit with
> 1. MICVDD enabled and 2. The pm_runtime_reference being unbalanced.
> 
> MICVDD still being enabled caused the following oops when the
> regulator is released by the devm framework:
> 
> [ 2850.745757] [ cut here ]
> [ 2850.745827] WARNING: CPU: 2 PID: 2098 at drivers/regulator/core.c:2123 
> _regulator_put.part.0+0x19f/0x1b0
> [ 2850.745835] Modules linked in: extcon_arizona ...
> ...
> [ 2850.746909] Call Trace:
> [ 2850.746932]  regulator_put+0x2d/0x40
> [ 2850.746946]  release_nodes+0x22a/0x260
> [ 2850.746984]  __device_release_driver+0x190/0x240
> [ 2850.747002]  driver_detach+0xd4/0x120
> ...
> [ 2850.747337] ---[ end trace f455dfd7abd9781f ]---
> 
> Note this oops is just one of various theoretically possible races caused
> by the wrong ordering inside arizona_extcon_remove(), this fixes the
> ordering fixing all possible races, including the reported oops.
> 
> Reviewed-by: Andy Shevchenko 
> Acked-by: Charles Keepax 
> Signed-off-by: Hans de Goede 
> ---

Tested-by: Charles Keepax 

Thanks,
Charles

Re: [PATCH v4 05/13] extcon: arizona: Always use pm_runtime_get_sync() when we need the device to be awake

[Charles Keepax]

On Sat, Jan 23, 2021 at 01:13:05PM +0100, Hans de Goede wrote:
> Before this commit the extcon-arizona code was mixing pm_runtime_get()
> and pm_runtime_get_sync() in different places.
> 
> In all places where pm_runtime_get[_sync]() is called, the code
> makes use of the device immediately after the call.
> This means that we should always use pm_runtime_get_sync().
> 
> Reviewed-by: Andy Shevchenko 
> Signed-off-by: Hans de Goede 
> ---

Tested-by: Charles Keepax 

Thanks,
Charles

Re: [PATCH v4 06/13] ASoC/extcon: arizona: Move arizona jack code to sound/soc/codecs/arizona-jack.c

[Charles Keepax]

On Sat, Jan 23, 2021 at 01:13:06PM +0100, Hans de Goede wrote:
> The jack handling for arizona codecs is being refactored so that it is
> done directly by the codec drivers, instead of having an extcon-driver
> bind to a separate "arizona-extcon" child-device for this.
> 
> drivers/mfd/arizona-core.c has already been updated to no longer
> instantiate an "arizona-extcon" child-device for the arizona codecs.
> 
> This means that the "arizona-extcon" driver is no longer useful
> (there are no longer any devices for it to bind to).
> 
> This commit drops the extcon Kconfig / Makefile bits and moves
> drivers/extcon/extcon-arizona.c to sound/soc/codecs/arizona-jack.c .
> 
> This is a preparation patch for converting the arizona extcon-driver into
> a helper library for letting the arizona codec-drivers directly report jack
> state through the standard sound/soc/soc-jack.c functions.
> 
> Signed-off-by: Hans de Goede 
> ---

Acked-by: Charles Keepax 
Tested-by: Charles Keepax 

Thanks,
Charles

Re: [PATCH v4 05/13] extcon: arizona: Always use pm_runtime_get_sync() when we need the device to be awake

[Charles Keepax]

On Sat, Jan 23, 2021 at 01:13:05PM +0100, Hans de Goede wrote:
> Before this commit the extcon-arizona code was mixing pm_runtime_get()
> and pm_runtime_get_sync() in different places.
> 
> In all places where pm_runtime_get[_sync]() is called, the code
> makes use of the device immediately after the call.
> This means that we should always use pm_runtime_get_sync().
> 
> Reviewed-by: Andy Shevchenko 
> Signed-off-by: Hans de Goede 
> ---

Acked-by: Charles Keepax 

Thanks,
Charles

Re: [PATCH] regulator: core: Avoid debugfs: Directory ... already present! error

[Charles Keepax]

On Fri, Jan 22, 2021 at 07:32:50PM +0100, Hans de Goede wrote:
> Sometimes regulator_get() gets called twice for the same supply on the
> same device. This may happen e.g. when a framework / library is used
> which uses the regulator; and the driver itself also needs to enable
> the regulator in some cases where the framework will not enable it.
> 
> Commit ff268b56ce8c ("regulator: core: Don't spew backtraces on
> duplicate sysfs") already takes care of the backtrace which would
> trigger when creating a duplicate consumer symlink under
> /sys/class/regulator/regulator.%d in this scenario.
> 
> Commit c33d442328f5 ("debugfs: make error message a bit more verbose")
> causes a new error to get logged in this scenario:
> 
> [   26.938425] debugfs: Directory 'wm5102-codec-MICVDD' with parent 
> 'spi-WM510204:00-MICVDD' already present!
> 
> There is no _nowarn variant of debugfs_create_dir(), but we can detect
> and avoid this problem by checking the return value of the earlier
> sysfs_create_link_nowarn() call.
> 
> Add a check for the earlier sysfs_create_link_nowarn() failing with
> -EEXIST and skip the debugfs_create_dir() call in that case, avoiding
> this error getting logged.
> 
> Fixes: c33d442328f5 ("debugfs: make error message a bit more verbose")
> Cc: Charles Keepax 
> Signed-off-by: Hans de Goede 
> ---

Reviewed-by: Charles Keepax 

Thanks,
Charles

> - int err;
> + int err = 0;
>  
> @@ -1663,8 +1663,8 @@ static struct regulator *create_regulator(struct 
> regulator_dev *rdev,
>  
> - regulator->debugfs = debugfs_create_dir(supply_name,
> - rdev->debugfs);
> + if (err != -EEXIST)
> + regulator->debugfs = debugfs_create_dir(supply_name, 
> rdev->debugfs);

There is a slight oddity here in that if this regulator has
no struct device we will still get the warning. However, I
am totally not clear on when/why a regulator might not have a
dev, and am fairly sure it isn't common. So my vote would be
to cross that bridge if we ever come to it.

Thanks,
Charles

Re: [PATCH v2 08/12] ASoC: arizona-jack: convert into a helper library for codec drivers

[Charles Keepax]

On Thu, Jan 21, 2021 at 05:55:00PM +0100, Hans de Goede wrote:
> On 1/19/21 10:51 AM, Richard Fitzgerald wrote:
> > On 18/01/2021 17:24, Andy Shevchenko wrote:
> >> On Sun, Jan 17, 2021 at 6:06 PM Hans de Goede  wrote:
> 1. Keep the code as is, live with the debugfs error. This might be
> best for now, as I don't want to grow the scope of this series too much.
> I will go with this for the next version of this series (unless
> I receive feedback otherwise before I get around to posting the next
> version).

Thinking about this more, I seem to remember this is something
that has been discussed before, having the need to have
situations where a driver and the framework are both managing the
regulator at once on the same device.

I wonder if this commit was related to that:

commit ff268b56ce8c ("regulator: core: Don't spew backtraces on duplicate 
sysfs")

Apologies I don't have as much time as I normally would to look
into such issues at the moment, due to various internal company
things going on.

I do suspect that this option is the way to go though and if
there are issues of duplicates being created by the regulator
core those probably need to be resolved in there. But that can
probably be done separate from this series.

Thanks,
Charles

Re: [PATCH v2 08/12] ASoC: arizona-jack: convert into a helper library for codec drivers

[Charles Keepax]

On Fri, Jan 22, 2021 at 01:23:44PM +0100, Hans de Goede wrote:
> On 1/22/21 12:26 PM, Charles Keepax wrote:
> > On Thu, Jan 21, 2021 at 05:55:00PM +0100, Hans de Goede wrote:
> >> On 1/19/21 10:51 AM, Richard Fitzgerald wrote:
> >>> On 18/01/2021 17:24, Andy Shevchenko wrote:
> >>>> On Sun, Jan 17, 2021 at 6:06 PM Hans de Goede  
> >>>> wrote:
> >> Note there is a pretty big issue with the original code here, if
> >> the MICVDD DAPM pin is on for an internal-mic and then we run through the
> >> jack-detect mic-detect sequence, we end up setting
> >> bypass=true causing the micbias for the internal-mic to no longer
> >> be what was configured. IOW poking the bypass setting underneath the
> >> DAPM code is racy.
> >>
> > 
> > The regulator bypass code keeps an internal reference count. All
> > the users of the regulator need to allow bypass for it to be
> > placed into bypass mode, so I believe this can't happen.
> 
> Ah I did not know that, since the regulator_allow_bypass function
> takes a bool rather then having enable/disable variants I thought
> it would directly set the bypass, but you are right. So this is not
> a problem, good.
> 
> So this has made me look at the problem again and I believe that
> a much better solution is to simply re-use the MICVDD regulator-reference
> which has been regulator_get-ed by the dapm code when instantiating the:
> 
> SND_SOC_DAPM_REGULATOR_SUPPLY("MICVDD", 0, SND_SOC_DAPM_REGULATOR_BYPASS),
> 
> widget. So I plan to have a new patch in v3 of the series which replaces
> the devm_regulator_get with something like this:
> 
>   /*
>* There is a DAPM widget for the MICVDD regulator, since
>* the button-press detection has special requirements wrt
>* the regulator bypass settings we cannot directly
>* use snd_soc_component_force_enable_pin("MICVDD") /
>* snd_soc_component_disable_pin("MICVDD").
>*
>* Instead we lookup the widget's regulator reference here
>* and use that to directly control the regulator.
>* Both the regulator's enable and bypass settings are
>* ref-counted so this will not interfere with the DAPM use
>* of the regulator.
>*/
>   for_each_card_widgets(dapm->card, w) {
>   if (!strcmp(w->name, "MICVDD"))
>   info->micvdd_regulator = w->regulator;
>   break;
>   }
>   }
> 
> (note I've not tested this yet, but I expect this to work fine).
> 

Alas this won't work either. When I say reference count that
isn't quite a totally accurate reflection of the usage of the
function. When you call allow_bypass you are saying as this
consumer of the regulator I don't mind if it goes into bypass.
Then if all consumers agree the regulator will be put into
bypass. So it is comparing the reference count to the number of
consumers the regulator has to make a decision.

If you call allow_bypass independently from the jack detection
code and the ASoC framework on the same consumer, as you
describe here you will get bad effects.  For example the
regulator has two consumers, our CODEC driver and some other
device. If our codec driver calls allow_bypass twice, then
the regulator would go into bypass without the other consumer
having approved it would could be fatal to that device.

Thanks,
Charles

Re: [PATCH v2 08/12] ASoC: arizona-jack: convert into a helper library for codec drivers

[Charles Keepax]

the bypass when we only need MICVDD for button-press
> detection.
> 

Yeah we really don't want to force the micbias's to be regulated
during button detect, so I think this option has to go.

> Note there is a pretty big issue with the original code here, if
> the MICVDD DAPM pin is on for an internal-mic and then we run through the
> jack-detect mic-detect sequence, we end up setting
> bypass=true causing the micbias for the internal-mic to no longer
> be what was configured. IOW poking the bypass setting underneath the
> DAPM code is racy.
> 

The regulator bypass code keeps an internal reference count. All
the users of the regulator need to allow bypass for it to be
placed into bypass mode, so I believe this can't happen.

> Keeping in mind that switching to force_enable fixes the current racy code,
> as well as the KISS-ness of this solution, I personally prefer this option
> over option 1 as it makes the code cleaner and more correct.
> I could easily do this in a next version of this series if people agree
> with going this route.
> 

It is pretty problematic to loose the power benefits of the
button detect, for the sake of making the code a little cleaner.

Thanks,
Charles

Re: [PATCH v2 06/12] ASoC: arizona-jack: Move jack-detect variables to struct arizona_priv

[Charles Keepax]

On Sun, Jan 17, 2021 at 05:05:49PM +0100, Hans de Goede wrote:
> Move all the jack-detect variables from struct arizona_extcon_info to
> struct arizona_priv.
> 
> This is part of a patch series converting the arizona extcon driver into
> a helper library for letting the arizona codec-drivers directly report jack
> state through the standard sound/soc/soc-jack.c functions.
> 
> Signed-off-by: Hans de Goede 
> ---

Acked-by: Charles Keepax 

Thanks,
Charles

Re: [PATCH v2 03/12] ASoC: arizona-jack: Fix some issues when HPDET IRQ fires after the jack has been unplugged

[Charles Keepax]

On Sun, Jan 17, 2021 at 05:05:46PM +0100, Hans de Goede wrote:
> When the jack is partially inserted and then removed again it may be
> removed while the hpdet code is running. In this case the following
> may happen:
> 
> 1. The "JACKDET rise" or ""JACKDET fall" IRQ triggers
> 2. arizona_jackdet runs and takes info->lock
> 3. The "HPDET" IRQ triggers
> 4. arizona_hpdet_irq runs, blocks on info->lock
> 5. arizona_jackdet calls arizona_stop_mic() and clears info->hpdet_done
> 6. arizona_jackdet releases info->lock
> 7. arizona_hpdet_irq now can continue running and:
> 7.1 Calls arizona_start_mic() (if a mic was detected)
> 7.2 sets info->hpdet_done
> 
> Step 7 is undesirable / a bug:
> 7.1 causes the device to stay in a high power-state (with MICVDD enabled)
> 7.2 causes hpdet to not run on the next jack insertion, which in turn
> causes the EXTCON_JACK_HEADPHONE state to never get set
> 
> This fixes both issues by skipping these 2 steps when arizona_hpdet_irq
> runs after the jack has been unplugged.
> 
> Reviewed-by: Andy Shevchenko 
> Signed-off-by: Hans de Goede 
> ---

Acked-by: Charles Keepax 

Thanks,
Charles

Re: [PATCH v4 5/5] ASoC: Intel: bytcr_wm5102: Add machine driver for BYT/WM5102

[Charles Keepax]

On Wed, Jan 20, 2021 at 10:49:57PM +0100, Hans de Goede wrote:
> From: Pierre-Louis Bossart 
> 
> Add a new ASoc Machine driver for Intel Baytrail platforms with a
> Wolfson Microelectronics WM5102 codec.
> 
> This is based on a past contributions [1] from Paulo Sergio Travaglia
>  based on the Levono kernel [2] combined with
> insights in things like the speaker GPIO from the android-x86 android
> port for the Lenovo Yoga Tablet 2 1051F/L [3].
> 
> [1] 
> https://patchwork.kernel.org/project/alsa-devel/patch/593313f5.3636c80a.50e05.4...@mx.google.com/
> [2] 
> https://github.com/lenovo-yt2-dev/android_kernel_lenovo_baytrail/blob/cm-12.1/sound/soc/intel/board/byt_bl_wm5102.c
> [3] https://github.com/Kitsune/Android_Yoga_Tablet_2-1051F_Kernel
> 
> The original machine driver from the Android ports was a crude modified
> copy of bytcr_rt5640.c adjusted to work with the WM5102 codec.
> This version has been extensively reworked to:
> 
> 1. Remove all rt5640 related quirk handling. to the best of my knowledge
> this setup is only used on the Lenovo Yoga Tablet 2 series (8, 10 and 13
> inch models) which all use the same setup. So there is no need to deal
> with all the variations with which we need to deal on rt5640 boards.
> 
> 2. Rework clock handling, properly turn off the FLL and the platform-clock
> when they are no longer necessary and don't reconfigure the FLL
> unnecessarily when it is already running. This fixes a number of:
> "Timed out waiting for lock" warnings being logged.
> 
> 3. Add the GPIO controlled Speaker-VDD regulator as a DAPM_SUPPLY
> 
> This only adds the machine driver and ACPI hooks, the BYT-CR detection
> quirk which these devices need will be added in a separate patch.
> 
> BugLink: https://github.com/thesofproject/linux/issues/2485
> Co-authored-by: Pierre-Louis Bossart 
> Signed-off-by: Pierre-Louis Bossart 
> Reviewed-by: Andy Shevchenko 
> Signed-off-by: Hans de Goede 
> ---

Reviewed-by: Charles Keepax 

Thanks,
Charles

Re: [PATCH v4 3/5] mfd: arizona: Add support for ACPI enumeration of WM5102 connected over SPI

[Charles Keepax]

On Wed, Jan 20, 2021 at 10:49:55PM +0100, Hans de Goede wrote:
> The Intel Bay Trail (x86/ACPI) based Lenovo Yoga Tablet 2 series use
> a WM5102 codec connected over SPI.
> 
> Add support for ACPI enumeration to arizona-spi so that arizona-spi can
> bind to the codec on these tablets.
> 
> This is loosely based on an earlier attempt (for Android-x86) at this by
> Christian Hartmann, combined with insights in things like the speaker GPIO
> from the android-x86 android port for the Lenovo Yoga Tablet 2 1051F/L [1].
> 
> [1] https://github.com/Kitsune/Android_Yoga_Tablet_2-1051F_Kernel
> 
> Cc: Christian Hartmann 
> Reviewed-by: Andy Shevchenko 
> Signed-off-by: Hans de Goede 
> ---

Acked-by: Charles Keepax 

Thanks,
Charles

Re: [PATCH v3 2/5] mfd: arizona: Replace arizona_of_get_type() with device_get_match_data()

[Charles Keepax]

On Sun, Jan 17, 2021 at 10:22:49PM +0100, Hans de Goede wrote:
> Replace the custom arizona_of_get_type() function with the generic
> device_get_match_data() helper. Besides being a nice cleanup this
> also makes it easier to add support for binding to ACPI enumerated
> devices.
> 
> While at it also fix a possible NULL pointer deref of the id
> argument to the probe functions (this could happen on e.g. manual
> driver binding through sysfs).
> 
> Suggested-by: Andy Shevchenko 
> Signed-off-by: Hans de Goede 
> ---

Acked-by: Charles Keepax 

Thanks,
Charles

Re: [PATCH v2] net: macb: Correct usage of MACB_CAPS_CLK_HW_CHG flag

[Charles Keepax]

On Wed, Dec 30, 2020 at 05:44:41PM +0100, Andrew Lunn wrote:
> On Wed, Dec 30, 2020 at 10:33:09AM +0000, Charles Keepax wrote:
> > A new flag MACB_CAPS_CLK_HW_CHG was added and all callers of
> > macb_set_tx_clk were gated on the presence of this flag.
> > 
> > -   if (!clk)
> > + if (!bp->tx_clk || !(bp->caps & MACB_CAPS_CLK_HW_CHG))
> > 
> > However the flag was not added to anything other than the new
> > sama7g5_gem, turning that function call into a no op for all other
> > systems. This breaks the networking on Zynq.
> > 
> > The commit message adding this states: a new capability so that
> > macb_set_tx_clock() to not be called for IPs having this
> > capability
> > 
> > This strongly implies that present of the flag was intended to skip
> > the function not absence of the flag. Update the if statement to
> > this effect, which repairs the existing users.
> > 
> > Fixes: daafa1d33cc9 ("net: macb: add capability to not set the clock rate")
> > Suggested-by: Andrew Lunn 
> > Signed-off-by: Charles Keepax 
> 
> Reviewed-by: Andrew Lunn 
> 
> Hi Charles
> 
> Since this is a fix, this should be based on the net tree. And you
> indicate this in the subject line with [PATCH net]. If this applies
> cleanly to net, Dave will probably just accept it, but please keep
> this in mind for any more submissions you make to netdev.

Apologies I totally forgot to add your Reviewed-by on the new
version. Too much turkey on the brain after the fattening season.

Thanks,
Charles

Re: [PATCH] ASoC: wm8962: Add optional mclk device tree binding

[Charles Keepax]

On Thu, Dec 17, 2020 at 10:27:40AM -0600, Adam Ford wrote:
> The driver can request an optional clock for mclk.
> Update the txt file to reflect this.
> 
> Suggested-by: Geert Uytterhoeven 
> Signed-off-by: Adam Ford 

Acked-by: Charles Keepax 

Thanks,
Charles

[PATCH net v3] net: macb: Correct usage of MACB_CAPS_CLK_HW_CHG flag

[Charles Keepax]

A new flag MACB_CAPS_CLK_HW_CHG was added and all callers of
macb_set_tx_clk were gated on the presence of this flag.

-   if (!clk)
+ if (!bp->tx_clk || !(bp->caps & MACB_CAPS_CLK_HW_CHG))

However the flag was not added to anything other than the new
sama7g5_gem, turning that function call into a no op for all other
systems. This breaks the networking on Zynq.

The commit message adding this states: a new capability so that
macb_set_tx_clock() to not be called for IPs having this
capability

This strongly implies that present of the flag was intended to skip
the function not absence of the flag. Update the if statement to
this effect, which repairs the existing users.

Fixes: daafa1d33cc9 ("net: macb: add capability to not set the clock rate")
Suggested-by: Andrew Lunn 
Signed-off-by: Charles Keepax 
---

Changes since v1:
 - Updated flag semantics to skip function, as appears to have been
   intended by the initial commit.

Changes since v2:
 - Adding "net" to the subject line

Thanks,
Charles

 drivers/net/ethernet/cadence/macb_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c 
b/drivers/net/ethernet/cadence/macb_main.c
index d5d910916c2e8..814a5b10141d1 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -467,7 +467,7 @@ static void macb_set_tx_clk(struct macb *bp, int speed)
 {
long ferr, rate, rate_rounded;
 
-   if (!bp->tx_clk || !(bp->caps & MACB_CAPS_CLK_HW_CHG))
+   if (!bp->tx_clk || (bp->caps & MACB_CAPS_CLK_HW_CHG))
return;
 
switch (speed) {
-- 
2.11.0

Re: [PATCH v2] net: macb: Correct usage of MACB_CAPS_CLK_HW_CHG flag

[Charles Keepax]

On Wed, Dec 30, 2020 at 05:44:41PM +0100, Andrew Lunn wrote:
> On Wed, Dec 30, 2020 at 10:33:09AM +0000, Charles Keepax wrote:
> > A new flag MACB_CAPS_CLK_HW_CHG was added and all callers of
> > macb_set_tx_clk were gated on the presence of this flag.
> > 
> > -   if (!clk)
> > + if (!bp->tx_clk || !(bp->caps & MACB_CAPS_CLK_HW_CHG))
> > 
> > However the flag was not added to anything other than the new
> > sama7g5_gem, turning that function call into a no op for all other
> > systems. This breaks the networking on Zynq.
> > 
> > The commit message adding this states: a new capability so that
> > macb_set_tx_clock() to not be called for IPs having this
> > capability
> > 
> > This strongly implies that present of the flag was intended to skip
> > the function not absence of the flag. Update the if statement to
> > this effect, which repairs the existing users.
> > 
> > Fixes: daafa1d33cc9 ("net: macb: add capability to not set the clock rate")
> > Suggested-by: Andrew Lunn 
> > Signed-off-by: Charles Keepax 
> 
> Reviewed-by: Andrew Lunn 
> 
> Hi Charles
> 
> Since this is a fix, this should be based on the net tree. And you
> indicate this in the subject line with [PATCH net]. If this applies
> cleanly to net, Dave will probably just accept it, but please keep
> this in mind for any more submissions you make to netdev.
> 

Is already based on the net tree, sorry wasn't aware I needed to add
that into the subject line. Will do a respin today.

Thanks,
Charles

[PATCH v2] net: macb: Correct usage of MACB_CAPS_CLK_HW_CHG flag

[Charles Keepax]

A new flag MACB_CAPS_CLK_HW_CHG was added and all callers of
macb_set_tx_clk were gated on the presence of this flag.

-   if (!clk)
+ if (!bp->tx_clk || !(bp->caps & MACB_CAPS_CLK_HW_CHG))

However the flag was not added to anything other than the new
sama7g5_gem, turning that function call into a no op for all other
systems. This breaks the networking on Zynq.

The commit message adding this states: a new capability so that
macb_set_tx_clock() to not be called for IPs having this
capability

This strongly implies that present of the flag was intended to skip
the function not absence of the flag. Update the if statement to
this effect, which repairs the existing users.

Fixes: daafa1d33cc9 ("net: macb: add capability to not set the clock rate")
Suggested-by: Andrew Lunn 
Signed-off-by: Charles Keepax 
---

Changes since v1:
 - Updated flag semantics to skip function, as appears to have been
   intended by the initial commit.

Thanks,
Charles

 drivers/net/ethernet/cadence/macb_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c 
b/drivers/net/ethernet/cadence/macb_main.c
index d5d910916c2e8..814a5b10141d1 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -467,7 +467,7 @@ static void macb_set_tx_clk(struct macb *bp, int speed)
 {
long ferr, rate, rate_rounded;
 
-   if (!bp->tx_clk || !(bp->caps & MACB_CAPS_CLK_HW_CHG))
+   if (!bp->tx_clk || (bp->caps & MACB_CAPS_CLK_HW_CHG))
return;
 
switch (speed) {
-- 
2.11.0

Re: [PATCH 01/14] mfd: arizona: Add jack pointer to struct arizona

[Charles Keepax]

On Tue, Dec 29, 2020 at 02:57:38PM +0100, Hans de Goede wrote:
> On 12/29/20 2:06 PM, Charles Keepax wrote:
> > On Mon, Dec 28, 2020 at 04:28:07PM +, Mark Brown wrote:
> >> On Mon, Dec 28, 2020 at 02:16:04PM +0100, Hans de Goede wrote:
> >>
> >>> And more in general AFAIK extcon is sort of deprecated and it is
> >>> not advised to use it for new code. I would esp. not expect it to
> >>> be used for new jack-detection code since we already have standard
> >>> uAPI support for that through sound/core/jack.c .
> >>
> >> Has Android been fixed to use the ALSA/input layer interfaces?  That's
> >> why that code is there, long term the goal was to have ALSA generate
> >> extcon events too so userspace could fall over to using that.  The basic
> >> thing at the time was that nobody liked any of the existing interfaces
> >> (the input layer thing is a total bodge stemming from it having been
> >> easy to hack in a key for GPIO detection and using ALSA controls means
> >> having to link against alsa-lib which is an awful faff for system level
> >> UI stuff) and there were three separate userspace interfaces used by
> >> different software stacks which needed to be joined together, extcon was
> >> felt to be a bit more designed and is a superset so that was the
> >> direction we were heading in.
> > 
> > Android has been updated to have the option to catch input events
> > for jack detection now.
> > 
> > I have always been slightly confused between extcon and the ALSA
> > jack reporting and have been unsure as to what is the longer term
> > plan here. I vaguely thought there was a gentle plan to move to
> > extcon, it is interesting to see Hans basically saying the
> > opposite that extcon is intended to be paritially deprecated. I
> > assume you just mean with respect to audio jacks, not other
> > connector types?
> 
> No I mean that afaik extcon is being deprecated in general. Extcon
> is mostly meant for kernel internal use, to allow things like
> charger-type-detection done by e.g. a fsa micro-usb mux or a
> Type-C PD controller to be hooked up to the actual charger chip
> and set the input-current-limit based on this.
> 

Fascinating thanks for taking the time to write such detailed
answers. I thought it was mostly intended for user-space usage,
but I guess I never really thought through that most of this
stuff you don't really need to know from user-space.

> > I would agree with Mark though that if extcon exists for external
> > connectors it seems odd that audio jacks would have their own
> > special way rather than just using the connector stuff.
> 
> Well as I said above in me experience the extcon code is (was) mostly
> meant for kernel internal use. The sysfs API is more of a debugging
> tool then anything else (IMHO).
> 
> Also the kernel has support for a lot of sound devices, including
> many with jack-detection support. Yet a grep for EXTCON_JACK_HEADPHONE
> over the entire mainline kernel tree shows that only extcon-arizona.c
> is using it. So given that we have dozens of drivers providing jack
> functionality through the sound/core/jack.c core and only 1 driver
> using the extcon interface I believe that the ship on how to export
> this to userspace has long sailed, since most userspace code will
> clearly expect the sound/core/jack.c way of doing things to be used.
> 
> Arguably we should/could maybe even drop the extcon part of extcon-arizona.c
> but I did not do that as I did not want to regress existing userspace
> code which may depend on this (on specific embedded/android devices).
> 

All reasonable arguments, with Android now supporting input
events for jacks I guess there would be no need for us to use
extcon for future devices.

There is maybe more argument for porting the Arizona code across
anyways, since for a long time Android didn't properly support extcon
either. It supported the earlier out of tree switch stuff, extcon
had a switch compatibility mode, but that didn't actually work I
think due to android hard coding some sysfs naming or something
(memory is a little fuzzy on the details was a while ago now).

I think extcon support was fixed in Android at about the same time
the support for input events was added. So it might be harmless but
someone probably needs to go and check the timeline before we go
changing stuff.

Thanks,
Charles

Re: [PATCH 13/14] ASoC: Intel: bytcr_wm5102: Add machine driver for BYT/WM5102

[Charles Keepax]

On Sun, Dec 27, 2020 at 10:12:31PM +0100, Hans de Goede wrote:
> From: Pierre-Louis Bossart 
> 
> Add a new ASoc Machine driver for Intel Baytrail platforms with a
> Wolfson Microelectronics WM5102 codec.
> 
> This is based on a past contributions [1] from Paulo Sergio Travaglia
>  based on the Levono kernel [2] combined with
> insights in things like the speaker GPIO from the android-x86 android
> port for the Lenovo Yoga Tablet 2 1051F/L [3].
> 
> [1] 
> https://patchwork.kernel.org/project/alsa-devel/patch/593313f5.3636c80a.50e05.4...@mx.google.com/
> [2] 
> https://github.com/lenovo-yt2-dev/android_kernel_lenovo_baytrail/blob/cm-12.1/sound/soc/intel/board/byt_bl_wm5102.c
> [3] https://github.com/Kitsune/Android_Yoga_Tablet_2-1051F_Kernel
> 
> The original machine driver from the Android ports was a crude modified
> copy of bytcr_rt5640.c adjusted to work with the WM5102 codec.
> This version has been extensively reworked to:
> 
> 1. Remove all rt5640 related quirk handling. to the best of my knowledge
> this setup is only used on the Lenovo Yoga Tablet 2 series (8, 10 and 13
> inch models) which all use the same setup. So there is no need to deal
> with all the variations with which we need to deal on rt5640 boards.
> 
> 2. Rework clock handling, properly turn off the FLL and the platform-clock
> when they are no longer necessary and don't reconfigure the FLL
> unnecessarily when it is already running. This fixes a number of:
> "Timed out waiting for lock" warnings being logged.
> 
> 3. Add the GPIO controlled Speaker-VDD regulator as a DAPM_SUPPLY
> 
> This only adds the machine driver and ACPI hooks, the BYT-CR detection
> quirk which these devices need will be added in a separate patch.
> 
> Co-authored-by: Pierre-Louis Bossart 
> Signed-off-by: Pierre-Louis Bossart 
> Signed-off-by: Hans de Goede 
> ---

Just a couple really minor comments.

> +static int byt_wm5102_prepare_and_enable_pll1(struct snd_soc_dai *codec_dai, 
> int rate)
> +{
> + struct snd_soc_component *codec_component = codec_dai->component;
> + int sr_mult = ((rate % 4000) == 0) ?
> + (WM5102_MAX_SYSCLK_4K / rate) :
> + (WM5102_MAX_SYSCLK_11025 / rate);
> + int ret;
> +
> + /* Reset FLL1 */
> + snd_soc_dai_set_pll(codec_dai, WM5102_FLL1_REFCLK, 
> ARIZONA_FLL_SRC_NONE, 0, 0);
> + snd_soc_dai_set_pll(codec_dai, WM5102_FLL1, ARIZONA_FLL_SRC_NONE, 0, 0);
> +
> + /* Configure the FLL1 PLL before selecting it */
> + ret = snd_soc_dai_set_pll(codec_dai, WM5102_FLL1, ARIZONA_CLK_SRC_MCLK1,
> +   MCLK_FREQ, rate * sr_mult);
> + if (ret) {
> + dev_err(codec_component->dev, "Error setting PLL: %d\n", ret);
> + return ret;
> + }
> +
> + ret = snd_soc_component_set_sysclk(codec_component, ARIZONA_CLK_SYSCLK,
> +ARIZONA_CLK_SRC_FLL1, rate * sr_mult,
> +SND_SOC_CLOCK_IN);
> + if (ret) {
> + dev_err(codec_component->dev, "Error setting ASYNCCLK: %d\n", 
> ret);

Error message should say SYSCLK not ASYNCCLK.

> + return ret;
> + }
> +
> + ret = snd_soc_component_set_sysclk(codec_component, ARIZONA_CLK_OPCLK, 
> 0,
> +rate * sr_mult, SND_SOC_CLOCK_OUT);
> + if (ret) {
> + dev_err(codec_component->dev, "Error setting OPCLK: %d\n", ret);
> + return ret;
> + }

OPCLK is a clock that can be outputted on the CODECs GPIOs. Is
that being used to clock some external component? If so it should
be added to the DAPM graph, if not you might as well remove this
call.

> +
> + ret = snd_soc_dai_set_sysclk(codec_dai, ARIZONA_CLK_SYSCLK,
> +  rate * 512, SND_SOC_CLOCK_IN);
> + if (ret) {
> + dev_err(codec_component->dev, "Error setting clock: %d\n", ret);
> + return ret;
> + }
> +

The rate you set here doesn't actually matter, on wm5102 this
just links the DAI to a specific clock domain and as they all
default to SYSCLK you can omit this call if you want. Although no
harm is caused by leaving it in.

Thanks,
Charles

Re: [PATCH 01/14] mfd: arizona: Add jack pointer to struct arizona

[Charles Keepax]

On Mon, Dec 28, 2020 at 04:28:07PM +, Mark Brown wrote:
> On Mon, Dec 28, 2020 at 02:16:04PM +0100, Hans de Goede wrote:
> 
> > And more in general AFAIK extcon is sort of deprecated and it is
> > not advised to use it for new code. I would esp. not expect it to
> > be used for new jack-detection code since we already have standard
> > uAPI support for that through sound/core/jack.c .
> 
> Has Android been fixed to use the ALSA/input layer interfaces?  That's
> why that code is there, long term the goal was to have ALSA generate
> extcon events too so userspace could fall over to using that.  The basic
> thing at the time was that nobody liked any of the existing interfaces
> (the input layer thing is a total bodge stemming from it having been
> easy to hack in a key for GPIO detection and using ALSA controls means
> having to link against alsa-lib which is an awful faff for system level
> UI stuff) and there were three separate userspace interfaces used by
> different software stacks which needed to be joined together, extcon was
> felt to be a bit more designed and is a superset so that was the
> direction we were heading in.

Android has been updated to have the option to catch input events
for jack detection now.

I have always been slightly confused between extcon and the ALSA
jack reporting and have been unsure as to what is the longer term
plan here. I vaguely thought there was a gentle plan to move to
extcon, it is interesting to see Hans basically saying the
opposite that extcon is intended to be paritially deprecated. I
assume you just mean with respect to audio jacks, not other
connector types?

I would agree with Mark though that if extcon exists for external
connectors it seems odd that audio jacks would have their own
special way rather than just using the connector stuff.

Thanks,
Charles

Re: [PATCH 09/14] extcon: arizona: Add arizona_set_extcon_state() helper

[Charles Keepax]

On Sun, Dec 27, 2020 at 10:12:27PM +0100, Hans de Goede wrote:
> All the callers of extcon_set_state_sync() log an error on failure,
> without doing any further error-handling (as there is nothing they
> can do about the error).
> 
> Factor this out into a helper to remove some duplicate code.
> 
> Signed-off-by: Hans de Goede 
> ---
>  drivers/extcon/extcon-arizona.c | 47 -
>  1 file changed, 17 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/extcon/extcon-arizona.c b/drivers/extcon/extcon-arizona.c
> index 145ca5cd40d5..d5b3231744f9 100644
> --- a/drivers/extcon/extcon-arizona.c
> +++ b/drivers/extcon/extcon-arizona.c
> @@ -595,6 +595,16 @@ static int arizona_hpdet_do_id(struct 
> arizona_extcon_info *info, int *reading,
>   return 0;
>  }
>  
> +static void arizona_set_extcon_state(struct arizona_extcon_info *info,
> +  unsigned int id, bool state)
> +{
> + int ret;
> +
> + ret = extcon_set_state_sync(info->edev, id, state);
> + if (ret)
> + dev_err(info->arizona->dev, "Failed to set extcon state: %d\n", 
> ret);
> +}

Would be nice to also print which ID it is that is failing,
would help to narrow things down since we lose the customer error
messages for each case.

Thanks,
Charles

Re: [PATCH 08/14] extcon: arizona: Fix flags parameter to the gpiod_get("wlf,micd-pol") call

[Charles Keepax]

On Sun, Dec 27, 2020 at 10:12:26PM +0100, Hans de Goede wrote:
> The initial value of the GPIO should match the info->micd_modes[0].gpio
> value. arizona_extcon_probe() already stores the necessary flag in a
> mode variable, but instead of passing mode as flags to the gpiod_get()
> it was using a hardcoded GPIOD_OUT_LOW.
> 
> Signed-off-by: Hans de Goede 
> ---

Acked-by: Charles Keepax 

Thanks,
Charles

Re: [PATCH 06/14] extcon: arizona: Fix various races on driver unbind

[Charles Keepax]

On Sun, Dec 27, 2020 at 10:12:24PM +0100, Hans de Goede wrote:
> We must free/disable all interrupts and cancel all pending works
> before doing further cleanup.
> 
> Before this commit arizona_extcon_remove() was doing several
> register writes to shut things down before disabling the IRQs
> and it was cancelling only 1 of the 3 different works used.
> 
> Move all the register-writes shutting things down to after
> the disabling of the IRQs and add the 2 missing
> cancel_delayed_work_sync() calls.
> 
> This fixes various possible races on driver unbind. One of which
> would always trigger on devices using the mic-clamp feature for
> jack detection. The ARIZONA_MICD_CLAMP_MODE_MASK update was
> done before disabling the IRQs, causing:
> 1. arizona_jackdet() to run
> 2. detect a jack being inserted (clamp disabled means jack inserted)
> 3. call arizona_start_mic() which:
> 3.1 Enables the MICVDD regulator
> 3.2 takes a pm_runtime_reference
> 
> And this was all happening after the ARIZONA_MICD_ENA bit clearing,
> which would undo 3.1 and 3.2 because the ARIZONA_MICD_CLAMP_MODE_MASK
> update was being done after the ARIZONA_MICD_ENA bit clearing.
> 
> So this means that arizona_extcon_remove() would exit with
> 1. MICVDD enabled and 2. The pm_runtime_reference being unbalanced.
> 
> MICVDD still being enabled caused the following oops when the
> regulator is released by the devm framework:
> 
> [ 2850.745757] [ cut here ]
> [ 2850.745827] WARNING: CPU: 2 PID: 2098 at drivers/regulator/core.c:2123 
> _regulator_put.part.0+0x19f/0x1b0
> [ 2850.745835] Modules linked in: extcon_arizona ...
> ...
> [ 2850.746909] Call Trace:
> [ 2850.746932]  regulator_put+0x2d/0x40
> [ 2850.746946]  release_nodes+0x22a/0x260
> [ 2850.746984]  __device_release_driver+0x190/0x240
> [ 2850.747002]  driver_detach+0xd4/0x120
> ...
> [ 2850.747337] ---[ end trace f455dfd7abd9781f ]---
> 
> Note this oops is just one of various theoretically possible races caused
> by the wrong ordering inside arizona_extcon_remove(), this fixes the
> ordering fixing all possible races, including the reported oops.
> 
> Signed-off-by: Hans de Goede 
> ---

Sorry yes there are a few rough corners on the extcon stuff, I
have clean up series I have been working on as part of
upstreaming the Madera extcon hopefully I will get that sent out
one day.

Acked-by: Charles Keepax 

Thanks,
Charles

Re: [PATCH 07/14] extcon: arizona: Fix modalias

[Charles Keepax]

On Sun, Dec 27, 2020 at 10:12:25PM +0100, Hans de Goede wrote:
> Fix the modalias so that the driver will be loaded automatically. The
> module's name is "extcon-arizona", following other extcon module-names.
> 
> But the driver's and platform-device's name is "arizona-extcon" and the
> modalias must match that.
> 
> Signed-off-by: Hans de Goede 
> ---

Acked-by: Charles Keepax 

Thanks,
Charles

Re: [PATCH 04/14] mfd: arizona: Allow building arizona MFD-core as module

[Charles Keepax]

On Sun, Dec 27, 2020 at 10:12:22PM +0100, Hans de Goede wrote:
> There is no reason why the arizona core,irq and codec model specific
> regmap bits cannot be build as a module. All they do is export symbols
> which are used by the arizona-spi and/or arizona-i2c modules, which
> themselves can be built as module.
> 
> Change the Kconfig and Makefile arizona bits so that the arizona MFD-core
> can be built as a module.
> 
> This is especially useful on x86 platforms with a WM5102 codec, this
> allows the arizona MFD driver necessary for the WM5102 codec to be
> enabled in generic distro-kernels without growing the base kernel-image
> size.
> 
> Signed-off-by: Hans de Goede 
> ---

I think this patch might still cause some issues. ASoC has an
idiom where the machine driver does a select on the necessary
CODEC drivers. Select doesn't take care of dependencies etc. So I
believe if you build the machine driver as built in, it then
selects the CODEC as built in. If you have the MFD as a module
the build then fails due to the the CODEC calling some Arizona
functions.

arizona_request_irq, arizona_free_irq, arizona_set_irq_wake

On Madera we made the equivalents inline functions to avoid the
issue, the same should work here.

include/linux/irqchip/irq-madera.h

Thanks,
Charles

Re: [PATCH 02/14] mfd: arizona: Add MODULE_SOFTDEP("pre: arizona_ldo1")

[Charles Keepax]

On Sun, Dec 27, 2020 at 10:12:20PM +0100, Hans de Goede wrote:
> The (shared) probing code of the arizona-i2c and arizona-spi modules
> takes the following steps during init:
> 
> 1. Call mfd_add_devices() for a set of early child-devices, this
> includes the arizona_ldo1 device which provides one of the
> core-regulators.
> 
> 2. Bulk enable the core-regulators.
> 
> 3. Read the device id.
> 
> 4. Call mfd_add_devices() for the other child-devices.
> 
> This sequence depends on 1. leading to not only the child-device
> being created, but also the driver for the child-device binding
> to it and registering its regulator.
> 
> This requires the arizona_ldo1 driver to be loaded before the
> shared probing code runs. Add a doftdep for this to both modules to
> ensure that this requirement is met.
> 
> Note this mirrors the existing MODULE_SOFTDEP("pre: wm8994_regulator")
> in the wm8994 code, which has a similar init sequence.
> 
> Signed-off-by: Hans de Goede 
> ---

Acked-by: Charles Keepax 

Thanks,
Charles

[PATCH] dt-bindings: (hwmon/pm6764tr) Add PM6764TR hwmon driver bindings

[Charles Hsu]

Document device tree bindings for STMicroelectronics PM6764tr Voltage
Regulator.

Signed-off-by: Charles Hsu 
---
 .../bindings/hwmon/pmbus/st,pm6764tr.yaml | 47 +++
 .../devicetree/bindings/trivial-devices.yaml  |  2 +
 2 files changed, 49 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/hwmon/pmbus/st,pm6764tr.yaml

diff --git a/Documentation/devicetree/bindings/hwmon/pmbus/st,pm6764tr.yaml 
b/Documentation/devicetree/bindings/hwmon/pmbus/st,pm6764tr.yaml
new file mode 100644
index ..b4b0d5614d8d
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/pmbus/st,pm6764tr.yaml
@@ -0,0 +1,47 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+
+$id: http://devicetree.org/schemas/hwmon/pmbus/st,pm6764tr.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: PM6764TR voltage regulator
+
+maintainers:
+  - Charles Hsu 
+
+description: |
+  The PM6764/66 is a high performance digital controller designed to power
+  Intel’s VR12.5 processors (PM6766) and memories (PM6764): all required
+  parameters are programmable through a PMBus™ interface.
+  The device utilizes digital technology to implement all control and
+  power management functions to provide maximum flexibility and performance.
+  The NVM is embedded to store custom configurations.
+
+  https://www.st.com/resource/en/data_brief/pm6764.pdf
+
+properties:
+  compatible:
+enum:
+  - st,pm6764tr
+
+  reg:
+maxItems: 1
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+i2c {
+#address-cells = <1>;
+#size-cells = <0>;
+
+pm6764tr@68 {
+compatible = "st,pm6764tr";
+reg = <0x68>;
+};
+};
diff --git a/Documentation/devicetree/bindings/trivial-devices.yaml 
b/Documentation/devicetree/bindings/trivial-devices.yaml
index bdc2dc318178..0a2dcd03220b 100644
--- a/Documentation/devicetree/bindings/trivial-devices.yaml
+++ b/Documentation/devicetree/bindings/trivial-devices.yaml
@@ -260,6 +260,8 @@ properties:
   - socionext,synquacer-tpm-mmio
 # i2c serial eeprom  (24cxx)
   - st,24c256
+# STMicroelectronics Voltage Regulator
+  - st,pm6764tr
 # Ambient Light Sensor with SMBUS/Two Wire Serial Interface
   - taos,tsl2550
 # Temperature Monitoring and Fan Control
-- 
2.25.1

Re: [PATCH] net: macb: Correct usage of MACB_CAPS_CLK_HW_CHG flag on Zynq

[Charles Keepax]

On Wed, Dec 23, 2020 at 08:24:41PM +0100, Andrew Lunn wrote:
> On Wed, Dec 23, 2020 at 06:41:44PM +0000, Charles Keepax wrote:
> > A new flag MACB_CAPS_CLK_HW_CHG was added and all callers of
> > macb_set_tx_clk were gated on the presence of this flag.
> > 
> > if (!bp->tx_clk || !(bp->caps & MACB_CAPS_CLK_HW_CHG))
> > 
> > However the flag was not added to anything other than the new
> > sama7g5_gem, turning that function call into a no op for all other
> > systems. This breaks the networking on Zynq.
> 
> I'm not sure this is the correct fix. I think the original patch might
> be broken. Look at the commit message wording:
> 
>   The patch adds a new
> capability so that macb_set_tx_clock() to not be called for IPs having
> this capability
> 
> So MACB_CAPS_CLK_HW_CHG disables something, not enables it. So i
> suspect this if statement is wrong and needs fixing.

Hmm... good spot, hopefully the original author can comment. The
flag name reads to me as clock rate can change, the commit message
definitely implies the opposite.

So it really depends if this function was intended to be skipped
for the sama7g5 gem or emac.

Thanks,
Charles

[PATCH] net: macb: Correct usage of MACB_CAPS_CLK_HW_CHG flag on Zynq

[Charles Keepax]

A new flag MACB_CAPS_CLK_HW_CHG was added and all callers of
macb_set_tx_clk were gated on the presence of this flag.

if (!bp->tx_clk || !(bp->caps & MACB_CAPS_CLK_HW_CHG))

However the flag was not added to anything other than the new
sama7g5_gem, turning that function call into a no op for all other
systems. This breaks the networking on Zynq.

This patch adds that flag to Zynq config, it is probably needed on other
systems as well but it is hard to know that without having access to
those systems, so I guess we just have to wait to see who else spots
breakage here.

Fixes: daafa1d33cc9 ("net: macb: add capability to not set the clock rate")
Signed-off-by: Charles Keepax 
---
 drivers/net/ethernet/cadence/macb_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c 
b/drivers/net/ethernet/cadence/macb_main.c
index d5d910916c2e8..590116b236ef7 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -4536,7 +4536,7 @@ static const struct macb_config zynqmp_config = {
 
 static const struct macb_config zynq_config = {
.caps = MACB_CAPS_GIGABIT_MODE_AVAILABLE | MACB_CAPS_NO_GIGABIT_HALF |
-   MACB_CAPS_NEEDS_RSTONUBR,
+   MACB_CAPS_NEEDS_RSTONUBR | MACB_CAPS_CLK_HW_CHG,
.dma_burst_length = 16,
.clk_init = macb_clk_init,
.init = macb_init,
-- 
2.11.0

Re: [PATCH] ASoC: meson: axg-tdm-interface: fix loopback

[Charles Keepax]

On Thu, Dec 17, 2020 at 04:08:12PM +0100, Jerome Brunet wrote:
> When the axg-tdm-interface was introduced, the backend DAI was marked as an
> endpoint when DPCM was walking the DAPM graph to find a its BE.
> 
> It is no longer the case since this
> commit 8dd26dff00c0 ("ASoC: dapm: Fix handling of custom_stop_condition on 
> DAPM graph walks")
> Because of this, when DPCM finds a BE it does everything it needs on the
> DAIs but it won't power up the widgets between the FE and the BE if there
> is no actual endpoint after the BE.
> 
> On meson-axg HWs, the loopback is a special DAI of the tdm-interface BE.
> It is only linked to the dummy codec since there no actual HW after it.
> >From the DAPM perspective, the DAI has no endpoint. Because of this, the TDM
> decoder, which is a widget between the FE and BE is not powered up.
> 
> >From the user perspective, everything seems fine but no data is produced.
> 
> Connecting the Loopback DAI to a dummy DAPM endpoint solves the problem.
> 
> Fixes: 8dd26dff00c0 ("ASoC: dapm: Fix handling of custom_stop_condition on 
> DAPM graph walks")
> Cc: Charles Keepax 
> Signed-off-by: Jerome Brunet 
> ---

Reviewed-by: Charles Keepax 

Thanks,
Charles

URGENT REPLY IS NEEDED FROM YOU

[CHARLES GOODMAN]

Attn: Beneficiary:

Congratulations!! Your payment has been approved and endorsed, with the
instruction and approvals are given from the Authorities Due to the
incessant scam activities going around the globe, the Authorities has
instructed our Financial Institution to use high Performance in
Banking System to set up a Personal Online Banking Account.

The sum of US$15,500,000.00 was deposited in our bank, The Management
has resolved to open Personal On-line Banking Account for you with our
bank and then give you the on-line access which will enable you to
check and make electronics wire transfer out to any part of the world
of your choice.

Kindly send the below information to enable us to set the account open for you.

Full Name:.. 
Full Address:.
Direct Cellphone Number:..
PASSPORT AND ID CARDS:.
YOUR OCCUPATION.
POSITION. 
DATE OF BIRTH..

Looking forward to your next letter


Regards

Thanks for banking with us,

Mr.Charles Goodman.
A/C Online Payment Officer,
US Capital Bank Branch North Carolina,
United States of America

Re: [PATCH] ASoC: dapm: remove widget from dirty list on free

[Charles Keepax]

On Sat, Dec 12, 2020 at 05:20:12PM -0800, Thomas Hebb wrote:
> A widget's "dirty" list_head, much like its "list" list_head, eventually
> chains back to a list_head on the snd_soc_card itself. This means that
> the list can stick around even after the widget (or all widgets) have
> been freed. Currently, however, widgets that are in the dirty list when
> freed remain there, corrupting the entire list and leading to memory
> errors and undefined behavior when the list is next accessed or
> modified.
> 
> I encountered this issue when a component failed to probe relatively
> late in snd_soc_bind_card(), causing it to bail out and call
> soc_cleanup_card_resources(), which eventually called
> snd_soc_dapm_free() with widgets that were still dirty from when they'd
> been added.
> 
> Fixes: db432b414e20 ("ASoC: Do DAPM power checks only for widgets changed 
> since last run")
> Cc: sta...@vger.kernel.org
> Signed-off-by: Thomas Hebb 
> ---
> 
>  sound/soc/soc-dapm.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
> index 7f87b449f950..148c095df27b 100644
> --- a/sound/soc/soc-dapm.c
> +++ b/sound/soc/soc-dapm.c
> @@ -2486,6 +2486,7 @@ void snd_soc_dapm_free_widget(struct 
> snd_soc_dapm_widget *w)
>   enum snd_soc_dapm_direction dir;
>  
>   list_del(>list);
> + list_del(>dirty);

I can't help but wonder if we should be taking the DAPM lock for
snd_soc_dapm_free_widgets. However your patch doesn't look like it
is making that any more scary and looks like we should be making
sure we remove the widget from the dirty list.

Reviewed-by: Charles Keepax 

Thanks,
Charles

Re: [PATCH] mfd: wm831x-auxadc: Prevent use after free in wm831x_auxadc_read_irq()

[Charles Keepax]

On Mon, Dec 14, 2020 at 02:59:10PM +0300, Dan Carpenter wrote:
> The "req" struct is always added to the "wm831x->auxadc_pending" list,
> but it's only removed from the list on the success path.  If a failure
> occurs then the "req" struct is freed but it's still on the list,
> leading to a use after free.
> 
> Fixes: 78bb3688ea18 ("mfd: Support multiple active WM831x AUXADC conversions")
> Signed-off-by: Dan Carpenter 
> ---

Acked-by: Charles Keepax 

Thanks,
Charles

[PATCH v6] hwmon: Add driver for STMicroelectronics PM6764 Voltage Regulator.

[Charles Hsu]

The output voltage use the MFR_READ_VOUT 0xD4
Vout value returned is linear11.

Signed-off-by: Charles Hsu 
---
v6:
 - Fix misspelling.
v5:
 - Add MAINTAINERS.
v4:
 - Add pm6764tr to Documentation/hwmon/index.rst.
v3:
 - Add Documentation(Documentation/hwmon/pm6764tr.rst).
 - Fix include order.
v2:
 - Fix formatting.
 - Remove pmbus_do_remove.
 - Change from .probe to .probe_new.
v1:
 - Initial patchset.
---
 Documentation/hwmon/index.rst|  1 +
 Documentation/hwmon/pm6764tr.rst | 32 ++
 MAINTAINERS  |  7 +++
 drivers/hwmon/pmbus/Kconfig  |  9 
 drivers/hwmon/pmbus/Makefile |  1 +
 drivers/hwmon/pmbus/pm6764tr.c   | 76 
 6 files changed, 126 insertions(+)
 create mode 100644 Documentation/hwmon/pm6764tr.rst
 create mode 100644 drivers/hwmon/pmbus/pm6764tr.c

diff --git a/Documentation/hwmon/index.rst b/Documentation/hwmon/index.rst
index b797db738225..b3ed0047543b 100644
--- a/Documentation/hwmon/index.rst
+++ b/Documentation/hwmon/index.rst
@@ -144,6 +144,7 @@ Hardware Monitoring Kernel Drivers
pc87360
pc87427
pcf8591
+   pm6764
pmbus
powr1220
pxe1610
diff --git a/Documentation/hwmon/pm6764tr.rst b/Documentation/hwmon/pm6764tr.rst
new file mode 100644
index ..4681780d3f0d
--- /dev/null
+++ b/Documentation/hwmon/pm6764tr.rst
@@ -0,0 +1,32 @@
+.. SPDX-License-Identifier: GPL-2.0-only
+
+Kernel driver pm6764tr
+==
+
+Supported chips:
+
+  * ST PM6764TR
+
+   Prefix: 'pm6764tr'
+
+   Addresses scanned: -
+
+   Datasheet: http://www.st.com/resource/en/data_brief/pm6764.pdf
+
+Authors:
+   
+
+Description:
+
+
+This driver supports the STMicroelectronics PM6764TR chip. The PM6764TR is a 
high
+performance digital controller designed to power Intel’s VR12.5 processors and 
memories.
+
+The device utilizes digital technology to implement all control and power 
management
+functions to provide maximum flexibility and performance. The NVM is embedded 
to store
+custom configurations. The PM6764TR device features up to 4-phase programmable 
operation.
+
+The PM6764TR supports power state transitions featuring VFDE, and programmable 
DPM
+maintaining the best efficiency over all loading conditions without 
compromising transient
+response. The device assures fast and independent protection against load 
overcurrent,
+under/overvoltage and feedback disconnections.
diff --git a/MAINTAINERS b/MAINTAINERS
index 2daa6ee673f7..0456a2101622 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -13893,6 +13893,13 @@ M: Logan Gunthorpe 
 S: Maintained
 F: drivers/dma/plx_dma.c
 
+PM6764TR DRIVER
+M: Charles Hsu 
+L: linux-hw...@vger.kernel.org
+S: Maintained
+F: Documentation/hwmon/pm6764tr.rst
+F: drivers/hwmon/pmbus/pm6764tr.c
+
 PM-GRAPH UTILITY
 M: "Todd E Brandt" 
 L: linux...@vger.kernel.org
diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
index a25faf69fce3..9c846facce9f 100644
--- a/drivers/hwmon/pmbus/Kconfig
+++ b/drivers/hwmon/pmbus/Kconfig
@@ -220,6 +220,15 @@ config SENSORS_MP2975
  This driver can also be built as a module. If so, the module will
  be called mp2975.
 
+config SENSORS_PM6764TR
+   tristate "ST PM6764TR"
+   help
+ If you say yes here you get hardware monitoring support for ST
+ PM6764TR.
+
+ This driver can also be built as a module. If so, the module will
+ be called pm6764tr.
+
 config SENSORS_PXE1610
tristate "Infineon PXE1610"
help
diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile
index 4c97ad0bd791..31ebdef5d4a6 100644
--- a/drivers/hwmon/pmbus/Makefile
+++ b/drivers/hwmon/pmbus/Makefile
@@ -25,6 +25,7 @@ obj-$(CONFIG_SENSORS_MAX31785)+= max31785.o
 obj-$(CONFIG_SENSORS_MAX34440) += max34440.o
 obj-$(CONFIG_SENSORS_MAX8688)  += max8688.o
 obj-$(CONFIG_SENSORS_MP2975)   += mp2975.o
+obj-$(CONFIG_SENSORS_PM6764TR) += pm6764tr.o
 obj-$(CONFIG_SENSORS_PXE1610)  += pxe1610.o
 obj-$(CONFIG_SENSORS_TPS40422) += tps40422.o
 obj-$(CONFIG_SENSORS_TPS53679) += tps53679.o
diff --git a/drivers/hwmon/pmbus/pm6764tr.c b/drivers/hwmon/pmbus/pm6764tr.c
new file mode 100644
index ..9531f370d7df
--- /dev/null
+++ b/drivers/hwmon/pmbus/pm6764tr.c
@@ -0,0 +1,76 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Hardware monitoring driver for STMicroelectronics digital controller 
PM6764TR
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include "pmbus.h"
+
+#define PM6764TR_PMBUS_READ_VOUT   0xD4
+
+static int pm6764tr_read_word_data(struct i2c_client *client, int page, int 
reg)
+{
+   int ret;
+
+   switch (reg) {
+   case PMBUS_VIRT_READ_VMON:
+   ret = pmbus_read_word_data(client, page, 
PM6764TR_PMBUS_READ_VOUT);
+   break;
+   default:
+

Re: [PATCH -next] regulator: wm831x-isink: convert comma to semicolon

[Charles Keepax]

On Fri, Dec 11, 2020 at 04:44:40PM +0800, Zheng Yongjun wrote:
> Replace a comma between expression statements by a semicolon.
> 
> Signed-off-by: Zheng Yongjun 
> ---

Acked-by: Charles Keepax 

Thanks,
Charles

Fordern Sie noch heute Ihre gespendeten Mittel an.

[Charles Jackson Jr]

-- 
Ihr E-Mail-Konto wurde für eine Spende von $ 3.500.000,00 USD für
wohltätige Zwecke ausgewählt. Antworten Sie auf die folgende E-Mail,
um weitere Informationen zu erhalten

E-Mail: michaelfis...@gmail.com

Re: wm8960: add DAC Slope switch

[Charles Keepax]

On Tue, Nov 24, 2020 at 06:23:13PM +0100, Lukas F. Hartmann wrote:
> The WM8960 DAC has a "DAC Slope" switch that can toggle between two
> different output filter curves. This patch adds support for it.
> 
> Signed-off-by: Lukas F. Hartmann 
> ---
> +static int wm8960_put_dacslope(struct snd_kcontrol *kcontrol,
> +struct snd_ctl_elem_value *ucontrol)
> +{
> + struct snd_soc_component *component = 
> snd_soc_kcontrol_component(kcontrol);
> + struct wm8960_priv *wm8960 = snd_soc_component_get_drvdata(component);
> + unsigned int val = ucontrol->value.integer.value[0];
> +
> + if (val > 1)
> + return -EINVAL;
> +
> + wm8960->dacslope = val;
> +
> + return snd_soc_component_update_bits(component, WM8960_DACCTL1,
> +    0x2, val<<1);

Is the DAC Slope switch not in DACCTL2?

Thanks,
Charles

Re: [PATCH v6] hwmon: Add driver for STMicroelectronics PM6764 Voltage Regulator

[Charles]

On 3/12/2020 下午11:48, Guenter Roeck wrote:

On Thu, Dec 03, 2020 at 08:34:32PM +0800, Charles wrote:
[ ... ]

It's really weird. I sent a mail to myself, and it looks good.
@@ -220,6 +220,15 @@ config SENSORS_MP2975
  This driver can also be built as a module. If so, the module will
  be called mp2975.
+config SENSORS_PM6764TR


But the above doesn't look good. The empty line is again missing.
It looks like your e-mail system manipulates/modifies the message
and removes that line.


I have applied my patch with "git am", and it did work.
Applying: Add-driver-for-STMicroelectronics-PM6764-Voltage-Regulator

I think I should use another e-mail system.

That may be a good idea. FWIW, I use msmtp, and git send-email.

Thanks,
Guenter


I will try this way to send my patch, thank you.

Charles

ANTWORT AUF SPENDENANSPRUCH...

[Charles W. Jackson Jr]

IHR E-MAIL-KONTO WURDE FÜR EINE SPENDE VON 3.500.000,00 USD FÜR CHARITY 
AUSGEWÄHLT. Antworten Sie auf die folgende E-Mail, um weitere Informationen zu 
erhalten

E-Mail: charlesjacksn...@gmail.com

-- 
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus

ANTWORT AUF SPENDENANSPRUCH

[Charles Jackson Jr]

IHR E-MAIL-KONTO WURDE FÜR EINE SPENDE VON 3.500.000,00 USD FÜR CHARITY 
AUSGEWÄHLT. Antworten Sie auf die folgende E-Mail, um weitere Informationen zu 
erhalten

E-Mail: charlesjacksn...@gmail.com

-- 
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus

ANTWORT AUF SPENDENANSPRUCH

[Charles Jackson Jr]

IHR E-MAIL-KONTO WURDE FÜR EINE SPENDE VON 3.500.000,00 USD FÜR CHARITY 
AUSGEWÄHLT. Antworten Sie auf die folgende E-Mail, um weitere Informationen zu 
erhalten

E-Mail: charlesjacksn...@gmail.com

-- 
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus

ANTWORT AUF SPENDENANSPRUCH

[Charles Jackson Jr]

IHR E-MAIL-KONTO WURDE FÜR EINE SPENDE VON 3.500.000,00 USD FÜR CHARITY 
AUSGEWÄHLT. Antworten Sie auf die folgende E-Mail, um weitere Informationen zu 
erhalten

E-Mail: charlesjacksn...@gmail.com

-- 
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus

Herzlichen Glückwunsch zum Gewinn !!!

[Charles Jackson Jr]

-- 

Ihr E-Mail-Konto wurde für eine Spende von $ 3.500.000,00 USD für
wohltätige Zwecke ausgewählt. Antworten Sie auf die folgende E-Mail,
um weitere Informationen zu erhalten

E-Mail: michaelfis...@gmail.com

ANTWORT AUF CHARITY-SPENDE

[Charles Jackson]

-- 
IHR E-MAIL-KONTO WURDE FÜR EINE SPENDE VON 3.500.000,00 USD FÜR
CHARITY AUSGEWÄHLT. Antworten Sie auf die folgende E-Mail, um weitere
Informationen zu erhalten

E-Mail: charlesjacksn...@gmail.com

Re: [PATCH v5] hwmon: Add driver for STMicroelectronics PM6764 Voltage Regulator

[Charles]

On 28/11/2020 上午12:10, Guenter Roeck wrote:

On Fri, Nov 27, 2020 at 09:59:01AM +0800, Charles wrote:

Add the pmbus driver for the STMicroelectronics pm6764 voltage regulator.

the output voltage use the MFR_READ_VOUT 0xD4
vout value returned is linear11

Signed-off-by: Charles Hsu 

This patch (again) didn't make it to any of the mailing lists.
Please try to find out why this is the case. I usually pick up
patches from https://patchwork.kernel.org/project/linux-hwmon/list/,
and may easily miss a patch if I can't find it there.


---

v5:
  - Add MAINTAINERS
  - Add a reference into trivial-devices.yaml
v4:
  - Add pm6764tr to Documentation/hwmon/index.rst.
v3:
  - Add Documentation(Documentation/hwmon/pm6764tr.rst).
  - Fix include order.
v2:
  - Fix formatting.
  - Remove pmbus_do_remove.
  - Change from .probe to .probe_new.
v1:
  - Initial patchset.

---

  .../devicetree/bindings/trivial-devices.yaml  |  2 +
  Documentation/hwmon/index.rst |  1 +
  Documentation/hwmon/pm6764tr.rst  | 33 
  MAINTAINERS   |  7 ++
  drivers/hwmon/pmbus/Kconfig   |  9 +++
  drivers/hwmon/pmbus/Makefile  |  1 +
  drivers/hwmon/pmbus/pm6764tr.c| 76 +++
  7 files changed, 129 insertions(+)
  create mode 100644 Documentation/hwmon/pm6764tr.rst
  create mode 100644 drivers/hwmon/pmbus/pm6764tr.c

diff --git a/Documentation/devicetree/bindings/trivial-devices.yaml 
b/Documentation/devicetree/bindings/trivial-devices.yaml
index ab623ba930d5..cdd7bdb6abbb 100644
--- a/Documentation/devicetree/bindings/trivial-devices.yaml
+++ b/Documentation/devicetree/bindings/trivial-devices.yaml
@@ -348,6 +348,8 @@ properties:
- socionext,synquacer-tpm-mmio
  # i2c serial eeprom  (24cxx)
- st,24c256
+# SMBus/I2C Voltage Regulator
+  - st,pm6764tr
  # Ambient Light Sensor with SMBUS/Two Wire Serial Interface
- taos,tsl2550
  # 8-Channels, 12-bit ADC

This, like all devicetre changes, needs to be a separate patch.
Also, please make sure to copy dt maintainers and the dt mailing list
when you send that patch.



Thank you for your suggestions.

I will send that patch as soon as possible.



diff --git a/Documentation/hwmon/index.rst b/Documentation/hwmon/index.rst
index b797db738225..1bbd05e41de4 100644
--- a/Documentation/hwmon/index.rst
+++ b/Documentation/hwmon/index.rst
@@ -144,6 +144,7 @@ Hardware Monitoring Kernel Drivers
 pc87360
 pc87427
 pcf8591
+   pm6764tr
 pmbus
 powr1220
 pxe1610
diff --git a/Documentation/hwmon/pm6764tr.rst b/Documentation/hwmon/pm6764tr.rst
new file mode 100644
index ..5e8092e39297
--- /dev/null
+++ b/Documentation/hwmon/pm6764tr.rst
@@ -0,0 +1,33 @@
+.. SPDX-License-Identifier: GPL-2.0-only
+
+Kernel driver pm6764tr
+==
+
+Supported chips:
+
+  * ST PM6764TR
+
+Prefix: 'pm6764tr'
+
+Addresses scanned: -
+
+Datasheet: http://www.st.com/resource/en/data_brief/pm6764.pdf
+
+Authors:
+   
+
+Description:
+
+
+This driver supports the STMicroelectronics PM6764TR chip. The PM6764TR is a 
high
+performance digital controller designed to power Intel’s VR12.5 processors and 
memories.
+

Unrelated side note: I understand this means that you are forced to keep the
datasheet under wraps, which in turn means I can not suggest functionality
improvements since I don't have access to it. If the chip happens to support
per-rail telemetry, you might want to consider adding support for that in a
follow-up patch.


+The device utilizes digital technology to implement all control and power 
management
+functions to provide maximum flexibility and performance. The NVM is embedded 
to store
+custom configurations. The PM6764TR device features up to 4-phase programmable 
operation.
+
+The PM6764TR supports power state transitions featuring VFDE, and programmable 
DPM
+maintaining the best efficiency over all loading conditions without 
compromising transient
+response. The device assures fast and independent protectionagainstload 
overcurrent,

"protectionagainstload" -> "protection against load"


+under/overvoltage and feedback disconnections.
+

Drop empty line at end.


diff --git a/MAINTAINERS b/MAINTAINERS
index 94ac10a153c7..a3fea132c4ed 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -13904,6 +13904,13 @@ M: Logan Gunthorpe 
  S:Maintained
  F:drivers/dma/plx_dma.c

Add empty line.



There is an empty line here,

Should I add one more empty line?



+PM6764TR DRIVER
+M: Charles Hsu 
+L: linux-hw...@vger.kernel.org
+S: Maintained
+F: Documentation/hwmon/pm6764tr.rst
+F: drivers/hwmon/pmbus/pm6764tr.c
+
  PM-GRAPH UTILITY
  M:"Todd E Brandt" 
  L:linux...@vger.kernel.org
diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
index 

Re: [PATCH v4] hwmon: Add driver for STMicroelectronics PM6764 Voltage Regulator

[Charles]

On 22/11/2020 下午11:36, Guenter Roeck wrote:

On Wed, Nov 11, 2020 at 03:10:08PM +0800, Charles wrote:

Add the pmbus driver for the STMicroelectronics pm6764 voltage regulator.

the output voltage use the MFR_READ_VOUT 0xD4
vout value returned is linear11

Signed-off-by: Charles Hsu 

Please fix all checkpatch errors and warnings before resubmitting,
and make sure that your e-mail agent does not corrupt the patch.

Guenter



There is still a warning can't fix.

Device tree bindings should be their own patch, right?

I'm not sure which patch should I submit first.

WARNING: DT compatible string "pm6764tr" appears un-documented -- check 
./Documentation/devicetree/bindings/

#189: FILE: drivers/hwmon/pmbus/pm6764tr.c:60:

+    {.compatible = "pm6764tr"},



---

v4:
   - Add pm6764tr to Documentation/hwmon/index.rst.
v3:
   - Add Documentation(Documentation/hwmon/pm6764tr.rst).
   - Fix include order.
v2:
   - Fix formatting.
   - Remove pmbus_do_remove.
   - Change from .probe to .probe_new.
v1:
   - Initial patchset.
---

   Documentation/hwmon/index.rst    |  1 +
   Documentation/hwmon/pm6764tr.rst | 33 ++
   drivers/hwmon/pmbus/Kconfig  |  9 
   drivers/hwmon/pmbus/Makefile |  1 +
   drivers/hwmon/pmbus/pm6764tr.c   | 78 
   5 files changed, 122 insertions(+)
   create mode 100644 Documentation/hwmon/pm6764tr.rst
   create mode 100644 drivers/hwmon/pmbus/pm6764tr.c

+MODULE_DESCRIPTION("PMBus driver for  ST PM6764TR");
+MODULE_LICENSE("GPL");

diff --git a/Documentation/hwmon/index.rst b/Documentation/hwmon/index.rst
index b797db738225..1bbd05e41de4 100644
--- a/Documentation/hwmon/index.rst
+++ b/Documentation/hwmon/index.rst
@@ -144,6 +144,7 @@ Hardware Monitoring Kernel Drivers
      pc87360
      pc87427
      pcf8591
+   pm6764tr
      pmbus
      powr1220
      pxe1610
diff --git a/Documentation/hwmon/pm6764tr.rst
b/Documentation/hwmon/pm6764tr.rst
new file mode 100644
index ..5e8092e39297
--- /dev/null
+++ b/Documentation/hwmon/pm6764tr.rst
@@ -0,0 +1,33 @@
+.. SPDX-License-Identifier: GPL-2.0-only
+
+Kernel driver pm6764tr
+==
+
+Supported chips:
+
+  * ST PM6764TR
+
+    Prefix: 'pm6764tr'
+
+    Addresses scanned: -
+
+    Datasheet: http://www.st.com/resource/en/data_brief/pm6764.pdf
+
+Authors:
+    
+
+Description:
+
+
+This driver supports the STMicroelectronics PM6764TR chip. The PM6764TR
is a high
+performance digital controller designed to power Intel’s VR12.5
processors and memories.
+
+The device utilizes digital technology to implement all control and
power management
+functions to provide maximum flexibility and performance. The NVM is
embedded to store
+custom configurations. The PM6764TR device features up to 4-phase
programmable operation.
+
+The PM6764TR supports power state transitions featuring VFDE, and
programmable DPM
+maintaining the best efficiency over all loading conditions without
compromising transient
+response. The device assures fast and independent protectionagainstload
overcurrent,
+under/overvoltage and feedback disconnections.
+
diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
index a25faf69fce3..9c846facce9f 100644
--- a/drivers/hwmon/pmbus/Kconfig
+++ b/drivers/hwmon/pmbus/Kconfig
@@ -220,6 +220,15 @@ config SENSORS_MP2975
     This driver can also be built as a module. If so, the module will
     be called mp2975.

+config SENSORS_PM6764TR
+    tristate "ST PM6764TR"
+    help
+      If you say yes here you get hardware monitoring support for ST
+      PM6764TR.
+
+      This driver can also be built as a module. If so, the module will
+      be called pm6764tr.
+
   config SENSORS_PXE1610
   tristate "Infineon PXE1610"
   help
diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile
index 4c97ad0bd791..31ebdef5d4a6 100644
--- a/drivers/hwmon/pmbus/Makefile
+++ b/drivers/hwmon/pmbus/Makefile
@@ -25,6 +25,7 @@ obj-$(CONFIG_SENSORS_MAX31785)    += max31785.o
   obj-$(CONFIG_SENSORS_MAX34440)    += max34440.o
   obj-$(CONFIG_SENSORS_MAX8688)    += max8688.o
   obj-$(CONFIG_SENSORS_MP2975)    += mp2975.o
+obj-$(CONFIG_SENSORS_PM6764TR)    += pm6764tr.o
   obj-$(CONFIG_SENSORS_PXE1610)    += pxe1610.o
   obj-$(CONFIG_SENSORS_TPS40422)    += tps40422.o
   obj-$(CONFIG_SENSORS_TPS53679)    += tps53679.o
diff --git a/drivers/hwmon/pmbus/pm6764tr.c b/drivers/hwmon/pmbus/pm6764tr.c
new file mode 100644
index ..2ab68036bb0c
--- /dev/null
+++ b/drivers/hwmon/pmbus/pm6764tr.c
@@ -0,0 +1,78 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Hardware monitoring driver for STMicroelectronics digital controller
PM6764TR
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include "pmbus.h"
+
+#define PM6764TR_PMBUS_READ_VOUT    0xD4
+
+static int pm6764tr_read_word_data(struct i2c_client *clien

Re: [PATCH 14/16] mfd: wm8994: drop of_match_ptr from of_device_id table

[Charles Keepax]

On Fri, Nov 20, 2020 at 05:21:31PM +0100, Krzysztof Kozlowski wrote:
> The driver can match only via the DT table so the table should be always
> used and the of_match_ptr does not have any sense (this also allows ACPI
> matching via PRP0001, even though it is not relevant here).  This fixes
> compile warning (!CONFIG_OF on x86_64):
> 
>   drivers/mfd/wm8994-core.c:618:34: warning: ‘wm8994_of_match’ defined but 
> not used [-Wunused-const-variable=]
> 
> Signed-off-by: Krzysztof Kozlowski 
> ---

Acked-by: Charles Keepax 

Thanks,
Charles

Re: [PATCH] regmap: Fix order of regmap write log

[Charles Keepax]

On Thu, Nov 12, 2020 at 03:02:17PM +, Lucas Tanure wrote:
> _regmap_write can trigger a _regmap_select_page, which will call
> another _regmap_write that will be executed first, but the log shows
> the inverse order
> 
> Also, keep consistency with _regmap_read which only logs in case of
> success
> 
> Signed-off-by: Lucas Tanure 
> ---

Reviewed-by: Charles Keepax 

Thanks,
Charles

Re: [PATCH v1] spi: fix client driver breakages when using GPIO descriptors

[Charles Keepax]

On Wed, Nov 11, 2020 at 11:24:14AM -0500, Sven Van Asbroeck wrote:
> On Wed, Nov 11, 2020 at 10:48 AM Mark Brown  wrote:
> >
> > Applied to
> >
> >https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
> 
> Thank you !
> 
> Now that our minds are still focused on this subject, should
> commit 138c9c32f090 ("spi: spidev: Fix CS polarity if GPIO descriptors
> are used")
> be reverted?
> 
> This fixed spidev to deal with SPI_CS_HIGH on gpiod.
> But after our fix, its behaviour will probably be broken again.
> 
> Another candidate for revert is
> commit ada9e3fcc175 ("spi: dw: Correct handling of native chipselect")
> although I don't understand that code well enough to be sure.
> 
> Adding Charles Keepax.

Looks like the code has changed a fair amount since my patch. The
important detail from it was trying to clarify the semantics of the
controller->set_cs callback. That function is called with a boolean
argument and that argument could have two possible meanings:

1) True means apply a high logic level to the chip select line.
2) True mean apply chip select.

Under interpretation 2) the chip select line would be set to a
different logic level depending on if the device is active high or
active low.

If I remember correctly at the point of my patch the core had just
changed between the two a couple of times but now consistently did 1)
(and looks like it still does), my patch intended to updated the
spi-dw driver to match that, as my SPI had stopped working. I think
it then turned out, my patch broke some other use-cases and that
the bit in the IP basically had 2) semantics in hardware. Which is
what this patch fixed:

commit 9aea644ca17b ("spi: dw: Fix native CS being unset")

After that patch my patch is mostly replaced so I don't think it
would make any sense to revert my patch at this point, and I
don't think your patch will break the spi-dw driver. I don't
have easy access to the hardware right now to test, but I will
give it is quick run when that option becomes available to me
again.

Your fix looks good to me, but I suspect you do need to fix the
spidev stuff although I have haven't looked at that in detail.

Thanks,
Charles

[PATCH v4] hwmon: Add driver for STMicroelectronics PM6764 Voltage Regulator

[Charles]

Add the pmbus driver for the STMicroelectronics pm6764 voltage regulator.

the output voltage use the MFR_READ_VOUT 0xD4
vout value returned is linear11

Signed-off-by: Charles Hsu 
---

v4:
 - Add pm6764tr to Documentation/hwmon/index.rst.
v3:
 - Add Documentation(Documentation/hwmon/pm6764tr.rst).
 - Fix include order.
v2:
 - Fix formatting.
 - Remove pmbus_do_remove.
 - Change from .probe to .probe_new.
v1:
 - Initial patchset.
---

 Documentation/hwmon/index.rst    |  1 +
 Documentation/hwmon/pm6764tr.rst | 33 ++
 drivers/hwmon/pmbus/Kconfig  |  9 
 drivers/hwmon/pmbus/Makefile |  1 +
 drivers/hwmon/pmbus/pm6764tr.c   | 78 
 5 files changed, 122 insertions(+)
 create mode 100644 Documentation/hwmon/pm6764tr.rst
 create mode 100644 drivers/hwmon/pmbus/pm6764tr.c

diff --git a/Documentation/hwmon/index.rst b/Documentation/hwmon/index.rst
index b797db738225..1bbd05e41de4 100644
--- a/Documentation/hwmon/index.rst
+++ b/Documentation/hwmon/index.rst
@@ -144,6 +144,7 @@ Hardware Monitoring Kernel Drivers
    pc87360
    pc87427
    pcf8591
+   pm6764tr
    pmbus
    powr1220
    pxe1610
diff --git a/Documentation/hwmon/pm6764tr.rst 
b/Documentation/hwmon/pm6764tr.rst

new file mode 100644
index ..5e8092e39297
--- /dev/null
+++ b/Documentation/hwmon/pm6764tr.rst
@@ -0,0 +1,33 @@
+.. SPDX-License-Identifier: GPL-2.0-only
+
+Kernel driver pm6764tr
+==
+
+Supported chips:
+
+  * ST PM6764TR
+
+    Prefix: 'pm6764tr'
+
+    Addresses scanned: -
+
+    Datasheet: http://www.st.com/resource/en/data_brief/pm6764.pdf
+
+Authors:
+    
+
+Description:
+
+
+This driver supports the STMicroelectronics PM6764TR chip. The PM6764TR 
is a high
+performance digital controller designed to power Intel’s VR12.5 
processors and memories.

+
+The device utilizes digital technology to implement all control and 
power management
+functions to provide maximum flexibility and performance. The NVM is 
embedded to store
+custom configurations. The PM6764TR device features up to 4-phase 
programmable operation.

+
+The PM6764TR supports power state transitions featuring VFDE, and 
programmable DPM
+maintaining the best efficiency over all loading conditions without 
compromising transient
+response. The device assures fast and independent protectionagainstload 
overcurrent,

+under/overvoltage and feedback disconnections.
+
diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
index a25faf69fce3..9c846facce9f 100644
--- a/drivers/hwmon/pmbus/Kconfig
+++ b/drivers/hwmon/pmbus/Kconfig
@@ -220,6 +220,15 @@ config SENSORS_MP2975
   This driver can also be built as a module. If so, the module will
   be called mp2975.

+config SENSORS_PM6764TR
+    tristate "ST PM6764TR"
+    help
+      If you say yes here you get hardware monitoring support for ST
+      PM6764TR.
+
+      This driver can also be built as a module. If so, the module will
+      be called pm6764tr.
+
 config SENSORS_PXE1610
 tristate "Infineon PXE1610"
 help
diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile
index 4c97ad0bd791..31ebdef5d4a6 100644
--- a/drivers/hwmon/pmbus/Makefile
+++ b/drivers/hwmon/pmbus/Makefile
@@ -25,6 +25,7 @@ obj-$(CONFIG_SENSORS_MAX31785)    += max31785.o
 obj-$(CONFIG_SENSORS_MAX34440)    += max34440.o
 obj-$(CONFIG_SENSORS_MAX8688)    += max8688.o
 obj-$(CONFIG_SENSORS_MP2975)    += mp2975.o
+obj-$(CONFIG_SENSORS_PM6764TR)    += pm6764tr.o
 obj-$(CONFIG_SENSORS_PXE1610)    += pxe1610.o
 obj-$(CONFIG_SENSORS_TPS40422)    += tps40422.o
 obj-$(CONFIG_SENSORS_TPS53679)    += tps53679.o
diff --git a/drivers/hwmon/pmbus/pm6764tr.c b/drivers/hwmon/pmbus/pm6764tr.c
new file mode 100644
index ..2ab68036bb0c
--- /dev/null
+++ b/drivers/hwmon/pmbus/pm6764tr.c
@@ -0,0 +1,78 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Hardware monitoring driver for STMicroelectronics digital controller 
PM6764TR

+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include "pmbus.h"
+
+#define PM6764TR_PMBUS_READ_VOUT    0xD4
+
+static int pm6764tr_read_word_data(struct i2c_client *client, int page, 
int reg)

+{
+    int ret;
+
+    switch (reg) {
+    case PMBUS_VIRT_READ_VMON:
+        ret = pmbus_read_word_data(client, page, PM6764TR_PMBUS_READ_VOUT);
+        break;
+    default:
+        ret = -ENODATA;
+        break;
+    }
+    return ret;
+}
+
+static struct pmbus_driver_info pm6764tr_info = {
+    .pages = 1,
+    .format[PSC_VOLTAGE_IN] = linear,
+    .format[PSC_VOLTAGE_OUT] = vid,
+    .format[PSC_TEMPERATURE] = linear,
+    .format[PSC_CURRENT_OUT] = linear,
+    .format[PSC_POWER] = linear,
+    .func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_IIN |  PMBUS_HAVE_PIN |
+        PMBUS_HAVE_IOUT | PMBUS_HAVE_POUT | PMBUS_HAVE_VMON |
+        PMBUS_HAVE_STATUS_IOUT | PMBUS_HAVE_STATUS_VOUT |
+        PMBUS_HAVE

Re: [PATCH] hwmon: (pmbus) Add driver for STMicroelectronics PM6764 Voltage Regulator

[Charles Hsu]

On Wed, Nov 4, 2020 at 10:40 PM Guenter Roeck  wrote:
>
> On Wed, Nov 04, 2020 at 03:09:23PM +0800, Yungteng Hsu wrote:
> > Add the pmbus driver for the STMicroelectronics pm6764 voltage regulator.
> >
> > the output voltage use the MFR_READ_VOUT 0xD4
> > vout value returned is linear11
> >
> > Signed-off-by: Charles Hsu 
>
> Please use standard Linux kernel code formatting rules.
> Run checkpatch --strict on the patch and make sure it does not report
> any problems.
>
> total: 1 errors, 39 warnings, 3 checks, 96 lines checked
>
> is a bit much for a 74-line patch.
>

Okay, I got it.

> > ---
> >  drivers/hwmon/pmbus/Kconfig|  8 
> >  drivers/hwmon/pmbus/Makefile   |  1 +
> >  drivers/hwmon/pmbus/pm6764tr.c | 74 ++
> >  3 files changed, 83 insertions(+)
> >  create mode 100644 drivers/hwmon/pmbus/pm6764tr.c
> >
> > diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
> > index a25faf69fce3..2b487c401a4e 100644
> > --- a/drivers/hwmon/pmbus/Kconfig
> > +++ b/drivers/hwmon/pmbus/Kconfig
> > @@ -220,6 +220,14 @@ config SENSORS_MP2975
> >This driver can also be built as a module. If so, the module will
> >be called mp2975.
> >
> > +config SENSORS_PM6764TR
> > + tristate "PM6764TR"
> > + help
> > +  If you say yes here you get hardware monitoring support for PM6764TR.
> > +
> > +  This driver can also be built as a module. If so, the module will
> > +  be called pm6764tr.
> > +
> >  config SENSORS_PXE1610
> >   tristate "Infineon PXE1610"
> >   help
> > diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile
> > index 4c97ad0bd791..31ebdef5d4a6 100644
> > --- a/drivers/hwmon/pmbus/Makefile
> > +++ b/drivers/hwmon/pmbus/Makefile
> > @@ -25,6 +25,7 @@ obj-$(CONFIG_SENSORS_MAX31785) += max31785.o
> >  obj-$(CONFIG_SENSORS_MAX34440) += max34440.o
> >  obj-$(CONFIG_SENSORS_MAX8688) += max8688.o
> >  obj-$(CONFIG_SENSORS_MP2975) += mp2975.o
> > +obj-$(CONFIG_SENSORS_PM6764TR) += pm6764tr.o
> >  obj-$(CONFIG_SENSORS_PXE1610) += pxe1610.o
> >  obj-$(CONFIG_SENSORS_TPS40422) += tps40422.o
> >  obj-$(CONFIG_SENSORS_TPS53679) += tps53679.o
> > diff --git a/drivers/hwmon/pmbus/pm6764tr.c b/drivers/hwmon/pmbus/pm6764tr.c
> > new file mode 100644
> > index ..b125a1cbfea5
> > --- /dev/null
> > +++ b/drivers/hwmon/pmbus/pm6764tr.c
> > @@ -0,0 +1,74 @@
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
>
> Alphabetic include file order, please.
>

update include file order in v2.

> > +#include "pmbus.h"
> > +
> > +#define PM6764TR_PMBUS_READ_VOUT 0xD4
>
> tab after PM6764TR_PMBUS_READ_VOUT, please.
>
> I see you don't use any tabs in the patch. Please do.
>
> > +
> > +static int pm6764tr_read_word_data(struct i2c_client *client, int
> > page, int reg)
>
> Corrupted ?
>
> That makes me wonder if the patch was corrupted by your e-mail system,
> and if that system replaced tabs with spaces. Is that possible ?
>

This is corrupted by e-mail system,
and tabs are replaced with spaces.
I'll check the e-mail system setting and updated in v2.

> > +{
> > + int ret;
> > +
> > + switch (reg) {
> > + case PMBUS_VIRT_READ_VMON:
> > + ret = pmbus_read_word_data(client, page,
> > +   PM6764TR_PMBUS_READ_VOUT);
>
> That can be one line (line limit is now 100 columns).
>

fix to one line in v2.

> > + break;
> > + default:
> > + ret = -ENODATA;
> > + break;
> > + }
> > + return ret;
> > +}
> > +
> > +static struct pmbus_driver_info pm6764tr_info = {
> > + .pages = 1,
> > + .format[PSC_VOLTAGE_IN] = linear,
> > + .format[PSC_VOLTAGE_OUT] = vid,
> > + .format[PSC_TEMPERATURE] = linear,
> > + .format[PSC_CURRENT_OUT] = linear,
> > + .format[PSC_POWER] = linear,
> > + .func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_IIN |  PMBUS_HAVE_PIN |
> > +PMBUS_HAVE_IOUT | PMBUS_HAVE_POUT | PMBUS_HAVE_VMON |
> > + PMBUS_HAVE_STATUS_IOUT | PMBUS_HAVE_STATUS_VOUT |
> > + PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP,
> > +.read_word_data = pm6764tr_read_word_data,
> > +};
> > +
> > +static int pm6764tr_probe(struct i2c_client *client,
> > +  const struct i2c_device_id *id)
>
> Please use the new style probe function (probe_new).
>

Change form .probe to .probe_new in v2.

> > +{
> > + ret

Re: [PATCH 24/25] ASoC: wm8994: remove unnecessary CONFIG_PM_SLEEP

[Charles Keepax]

On Thu, Oct 29, 2020 at 03:43:00PM +0800, Coiby Xu wrote:
> SET_SYSTEM_SLEEP_PM_OPS has already took good care of CONFIG_PM_CONFIG.
> 
> Signed-off-by: Coiby Xu 
> ---
>  sound/soc/codecs/wm8994.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/sound/soc/codecs/wm8994.c b/sound/soc/codecs/wm8994.c
> index fc9ea198ac79..9294ad06f76d 100644
> --- a/sound/soc/codecs/wm8994.c
> +++ b/sound/soc/codecs/wm8994.c
> @@ -4656,7 +4656,6 @@ static int wm8994_remove(struct platform_device *pdev)
>   return 0;
>  }
>  
> -#ifdef CONFIG_PM_SLEEP
>  static int wm8994_suspend(struct device *dev)
>  {
>   struct wm8994_priv *wm8994 = dev_get_drvdata(dev);
> @@ -4681,7 +4680,6 @@ static int wm8994_resume(struct device *dev)
>  
>   return 0;
>  }
> -#endif
>  
>  static const struct dev_pm_ops wm8994_pm_ops = {
>   SET_SYSTEM_SLEEP_PM_OPS(wm8994_suspend, wm8994_resume)

Not sure this really makes sense, what is going to stop the
unused function warning if PM isn't configured?

Thanks,
Charles

[PATCH RESEND 1/2] mfd: madera: Add reset as part of resume

[Charles Keepax]

The DCVDD supply does not always power down when the CODEC enters
suspend, for example shared regulators or always-on regulators. In
these cases if a register is written back to the default value whilst
the CODEC is in suspend that register will retain the previous value.
As DCVDD never powered down, the register retains its old value and
as the cache sync only synchronises registers that differ from the
default the new value is never written out.

Ensure the registers are in the expected state after suspend by always
resetting the CODEC on resume. This also has the benefit of being
recommended by the datasheet for DCVDD supplies that take longer than
2mS to rise.

Signed-off-by: Charles Keepax 
---
 drivers/mfd/madera-core.c | 16 +++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/mfd/madera-core.c b/drivers/mfd/madera-core.c
index 4ed6ad8ce0020..a9c6f0833f327 100644
--- a/drivers/mfd/madera-core.c
+++ b/drivers/mfd/madera-core.c
@@ -291,6 +291,8 @@ static int __maybe_unused madera_runtime_resume(struct 
device *dev)
 
dev_dbg(dev, "Leaving sleep mode\n");
 
+   madera_enable_hard_reset(madera);
+
ret = regulator_enable(madera->dcvdd);
if (ret) {
dev_err(dev, "Failed to enable DCVDD: %d\n", ret);
@@ -300,7 +302,19 @@ static int __maybe_unused madera_runtime_resume(struct 
device *dev)
regcache_cache_only(madera->regmap, false);
regcache_cache_only(madera->regmap_32bit, false);
 
-   usleep_range(MADERA_RESET_MIN_US, MADERA_RESET_MAX_US);
+   madera_disable_hard_reset(madera);
+
+   if (!madera->pdata.reset) {
+   ret = madera_wait_for_boot(madera);
+   if (ret)
+   goto err;
+
+   ret = madera_soft_reset(madera);
+   if (ret) {
+   dev_err(dev, "Failed to reset: %d\n", ret);
+   goto err;
+   }
+   }
 
ret = madera_wait_for_boot(madera);
if (ret)
-- 
2.11.0

[PATCH RESEND 2/2] mfd: madera: Add special errata reset handling for cs47l15

[Charles Keepax]

An errata exists for cs47l15 where the reset must be handled
differently and removed before DCVDD is applied. A soft reset is used
for situations where a reset is required to reset state. This does
however, make this part unsuitable for DCVDD supplies with a rise time
greater than 2mS.

Signed-off-by: Charles Keepax 
---
 drivers/mfd/madera-core.c   | 25 -
 include/linux/mfd/madera/core.h |  1 +
 2 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/drivers/mfd/madera-core.c b/drivers/mfd/madera-core.c
index a9c6f0833f327..a2abc0094def7 100644
--- a/drivers/mfd/madera-core.c
+++ b/drivers/mfd/madera-core.c
@@ -38,6 +38,9 @@
 #define MADERA_RESET_MIN_US2000
 #define MADERA_RESET_MAX_US3000
 
+#define ERRATA_DCVDD_MIN_US1
+#define ERRATA_DCVDD_MAX_US15000
+
 static const char * const madera_core_supplies[] = {
"AVDD",
"DBVDD1",
@@ -291,7 +294,8 @@ static int __maybe_unused madera_runtime_resume(struct 
device *dev)
 
dev_dbg(dev, "Leaving sleep mode\n");
 
-   madera_enable_hard_reset(madera);
+   if (!madera->reset_errata)
+   madera_enable_hard_reset(madera);
 
ret = regulator_enable(madera->dcvdd);
if (ret) {
@@ -302,9 +306,12 @@ static int __maybe_unused madera_runtime_resume(struct 
device *dev)
regcache_cache_only(madera->regmap, false);
regcache_cache_only(madera->regmap_32bit, false);
 
-   madera_disable_hard_reset(madera);
+   if (madera->reset_errata)
+   usleep_range(ERRATA_DCVDD_MIN_US, ERRATA_DCVDD_MAX_US);
+   else
+   madera_disable_hard_reset(madera);
 
-   if (!madera->pdata.reset) {
+   if (!madera->pdata.reset || madera->reset_errata) {
ret = madera_wait_for_boot(madera);
if (ret)
goto err;
@@ -503,6 +510,8 @@ int madera_dev_init(struct madera *madera)
 */
switch (madera->type) {
case CS47L15:
+   madera->reset_errata = true;
+   break;
case CS47L35:
case CS47L90:
case CS47L91:
@@ -553,13 +562,19 @@ int madera_dev_init(struct madera *madera)
goto err_dcvdd;
}
 
+   if (madera->reset_errata)
+   madera_disable_hard_reset(madera);
+
ret = regulator_enable(madera->dcvdd);
if (ret) {
dev_err(dev, "Failed to enable DCVDD: %d\n", ret);
goto err_enable;
}
 
-   madera_disable_hard_reset(madera);
+   if (madera->reset_errata)
+   usleep_range(ERRATA_DCVDD_MIN_US, ERRATA_DCVDD_MAX_US);
+   else
+   madera_disable_hard_reset(madera);
 
regcache_cache_only(madera->regmap, false);
regcache_cache_only(madera->regmap_32bit, false);
@@ -667,7 +682,7 @@ int madera_dev_init(struct madera *madera)
 * It looks like a device we support. If we don't have a hard reset
 * we can now attempt a soft reset.
 */
-   if (!madera->pdata.reset) {
+   if (!madera->pdata.reset || madera->reset_errata) {
ret = madera_soft_reset(madera);
if (ret)
goto err_reset;
diff --git a/include/linux/mfd/madera/core.h b/include/linux/mfd/madera/core.h
index ad2c138105d4b..03a8a788424a2 100644
--- a/include/linux/mfd/madera/core.h
+++ b/include/linux/mfd/madera/core.h
@@ -186,6 +186,7 @@ struct madera {
struct regulator_bulk_data core_supplies[MADERA_MAX_CORE_SUPPLIES];
struct regulator *dcvdd;
bool internal_dcvdd;
+   bool reset_errata;
 
struct madera_pdata pdata;
 
-- 
2.11.0

Re: [PATCH] power: remove unneeded break

[Charles Keepax]

On Mon, Oct 19, 2020 at 11:59:37AM -0700, t...@redhat.com wrote:
> From: Tom Rix 
> 
> A break is not needed if it is preceded by a goto
> 
> Signed-off-by: Tom Rix 
> ---

Acked-by: Charles Keepax 

Thanks,
Charles

[PATCH] hwmon: (pmbus) Add driver for STMicroelectronics PM6764TR Voltage Regulator

[Charles Hsu]

Add the pmbus driver for the STMicroelectronics pm6764tr voltage regulator.

Signed-off-by: Charles Hsu 
---
 drivers/hwmon/pmbus/Kconfig|  8 
 drivers/hwmon/pmbus/Makefile   |  1 +
 drivers/hwmon/pmbus/pm6764tr.c | 76 ++
 3 files changed, 85 insertions(+)
 create mode 100644 drivers/hwmon/pmbus/pm6764tr.c

diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
index a25faf69fce3..e976997ee163 100644
--- a/drivers/hwmon/pmbus/Kconfig
+++ b/drivers/hwmon/pmbus/Kconfig
@@ -287,4 +287,12 @@ config SENSORS_ZL6100
   This driver can also be built as a module. If so, the module will
   be called zl6100.

+config SENSORS_PM6764TR
+ tristate "PM6764TR"
+ help
+  If you say yes here you get hardware monitoring support for PM6764TR.
+
+  This driver can also be built as a module. If so, the module will
+  be called pm6764tr.
+
 endif # PMBUS
diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile
index 4c97ad0bd791..bb89fcf9544d 100644
--- a/drivers/hwmon/pmbus/Makefile
+++ b/drivers/hwmon/pmbus/Makefile
@@ -32,3 +32,4 @@ obj-$(CONFIG_SENSORS_UCD9000) += ucd9000.o
 obj-$(CONFIG_SENSORS_UCD9200) += ucd9200.o
 obj-$(CONFIG_SENSORS_XDPE122) += xdpe12284.o
 obj-$(CONFIG_SENSORS_ZL6100) += zl6100.o
+obj-$(CONFIG_SENSORS_PM6764TR) += pm6764tr.o
\ No newline at end of file
diff --git a/drivers/hwmon/pmbus/pm6764tr.c b/drivers/hwmon/pmbus/pm6764tr.c
new file mode 100644
index ..e03b1441268e
--- /dev/null
+++ b/drivers/hwmon/pmbus/pm6764tr.c
@@ -0,0 +1,76 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include "pmbus.h"
+
+#define PM6764TR_PMBUS_READ_VOUT 0xD4
+
+static int pm6764tr_read_word_data(struct i2c_client *client, int
page, int reg)
+{
+ int ret;
+
+ switch (reg) {
+ case PMBUS_VIRT_READ_VMON:
+ ret = pmbus_read_word_data(client, page,
+   PM6764TR_PMBUS_READ_VOUT);
+ break;
+ default:
+ ret = -ENODATA;
+ break;
+ }
+ return ret;
+}
+
+static struct pmbus_driver_info pm6764tr_info = {
+ .pages = 1,
+ .format[PSC_VOLTAGE_IN] = linear,
+ .format[PSC_VOLTAGE_OUT] = vid,
+ .format[PSC_TEMPERATURE] = linear,
+ .format[PSC_CURRENT_OUT] = linear,
+ .format[PSC_POWER] = linear,
+ .func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_IIN |  PMBUS_HAVE_PIN |
+PMBUS_HAVE_IOUT | PMBUS_HAVE_POUT | PMBUS_HAVE_VMON |
+ PMBUS_HAVE_STATUS_IOUT | PMBUS_HAVE_STATUS_VOUT |
+ PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP,
+.read_word_data = pm6764tr_read_word_data,
+};
+
+static int pm6764tr_probe(struct i2c_client *client,
+  const struct i2c_device_id *id)
+{
+ return pmbus_do_probe(client, id, _info);
+}
+
+static const struct i2c_device_id pm6764tr_id[] = {
+ {"pm6764tr", 0},
+ {}
+};
+MODULE_DEVICE_TABLE(i2c, pm6764tr_id);
+
+static const struct of_device_id pm6764tr_of_match[] = {
+ {.compatible = "pm6764tr"},
+ {}
+};
+
+static struct i2c_driver pm6764tr_driver = {
+ .driver = {
+   .name = "pm6764tr",
+   .of_match_table = of_match_ptr(pm6764tr_of_match),
+   },
+ .probe = pm6764tr_probe,
+ .remove = pmbus_do_remove,
+ .id_table = pm6764tr_id,
+};
+
+module_i2c_driver(pm6764tr_driver);
+
+MODULE_AUTHOR("Charles Hsu");
+MODULE_DESCRIPTION("PMBus driver for ST PM6764TR");
+MODULE_LICENSE("GPL");
--
2.25.1

Re: [PATCH 1/8] ASoC: wm8350: use semicolons rather than commas to separate statements

[Charles Keepax]

On Sun, Oct 11, 2020 at 11:19:32AM +0200, Julia Lawall wrote:
> Replace commas with semicolons.  What is done is essentially described by
> the following Coccinelle semantic patch (http://coccinelle.lip6.fr/):
> 
> // 
> @@ expression e1,e2; @@
> e1
> -,
> +;
> e2
> ... when any
> // 
> 
> Signed-off-by: Julia Lawall 
> 
> ---

Acked-by: Charles Keepax 

Thanks,
Charles

Re: [PATCH 7/8] ASoC: madera: use semicolons rather than commas to separate statements

[Charles Keepax]

On Sun, Oct 11, 2020 at 11:19:38AM +0200, Julia Lawall wrote:
> Replace commas with semicolons.  What is done is essentially described by
> the following Coccinelle semantic patch (http://coccinelle.lip6.fr/):
> 
> // 
> @@ expression e1,e2; @@
> e1
> -,
> +;
> e2
> ... when any
> // 
> 
> Signed-off-by: Julia Lawall 
> 
> ---

Acked-by: Charles Keepax 

Thanks,
Charles

[PATCH 1/2] mfd: madera: Add reset as part of resume

[Charles Keepax]

The DCVDD supply does not always power down when the CODEC enters
suspend, for example shared regulators or always-on regulators. In
these cases if a register is written back to the default value whilst
the CODEC is in suspend that register will retain the previous value.
As DCVDD never powered down, the register retains its old value and
as the cache sync only synchronises registers that differ from the
default the new value is never written out.

Ensure the registers are in the expected state after suspend by always
resetting the CODEC on resume. This also has the benefit of being
recommended by the datasheet for DCVDD supplies that take longer than
2mS to rise.

Signed-off-by: Charles Keepax 
---
 drivers/mfd/madera-core.c | 16 +++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/mfd/madera-core.c b/drivers/mfd/madera-core.c
index 4ed6ad8ce0020..a9c6f0833f327 100644
--- a/drivers/mfd/madera-core.c
+++ b/drivers/mfd/madera-core.c
@@ -291,6 +291,8 @@ static int __maybe_unused madera_runtime_resume(struct 
device *dev)
 
dev_dbg(dev, "Leaving sleep mode\n");
 
+   madera_enable_hard_reset(madera);
+
ret = regulator_enable(madera->dcvdd);
if (ret) {
dev_err(dev, "Failed to enable DCVDD: %d\n", ret);
@@ -300,7 +302,19 @@ static int __maybe_unused madera_runtime_resume(struct 
device *dev)
regcache_cache_only(madera->regmap, false);
regcache_cache_only(madera->regmap_32bit, false);
 
-   usleep_range(MADERA_RESET_MIN_US, MADERA_RESET_MAX_US);
+   madera_disable_hard_reset(madera);
+
+   if (!madera->pdata.reset) {
+   ret = madera_wait_for_boot(madera);
+   if (ret)
+   goto err;
+
+   ret = madera_soft_reset(madera);
+   if (ret) {
+   dev_err(dev, "Failed to reset: %d\n", ret);
+   goto err;
+   }
+   }
 
ret = madera_wait_for_boot(madera);
if (ret)
-- 
2.11.0

[PATCH 2/2] mfd: madera: Add special errata reset handling for cs47l15

[Charles Keepax]

An errata exists for cs47l15 where the reset must be handled
differently and removed before DCVDD is applied. A soft reset is used
for situations where a reset is required to reset state. This does
however, make this part unsuitable for DCVDD supplies with a rise time
greater than 2mS.

Signed-off-by: Charles Keepax 
---
 drivers/mfd/madera-core.c   | 25 -
 include/linux/mfd/madera/core.h |  1 +
 2 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/drivers/mfd/madera-core.c b/drivers/mfd/madera-core.c
index a9c6f0833f327..a2abc0094def7 100644
--- a/drivers/mfd/madera-core.c
+++ b/drivers/mfd/madera-core.c
@@ -38,6 +38,9 @@
 #define MADERA_RESET_MIN_US2000
 #define MADERA_RESET_MAX_US3000
 
+#define ERRATA_DCVDD_MIN_US1
+#define ERRATA_DCVDD_MAX_US15000
+
 static const char * const madera_core_supplies[] = {
"AVDD",
"DBVDD1",
@@ -291,7 +294,8 @@ static int __maybe_unused madera_runtime_resume(struct 
device *dev)
 
dev_dbg(dev, "Leaving sleep mode\n");
 
-   madera_enable_hard_reset(madera);
+   if (!madera->reset_errata)
+   madera_enable_hard_reset(madera);
 
ret = regulator_enable(madera->dcvdd);
if (ret) {
@@ -302,9 +306,12 @@ static int __maybe_unused madera_runtime_resume(struct 
device *dev)
regcache_cache_only(madera->regmap, false);
regcache_cache_only(madera->regmap_32bit, false);
 
-   madera_disable_hard_reset(madera);
+   if (madera->reset_errata)
+   usleep_range(ERRATA_DCVDD_MIN_US, ERRATA_DCVDD_MAX_US);
+   else
+   madera_disable_hard_reset(madera);
 
-   if (!madera->pdata.reset) {
+   if (!madera->pdata.reset || madera->reset_errata) {
ret = madera_wait_for_boot(madera);
if (ret)
goto err;
@@ -503,6 +510,8 @@ int madera_dev_init(struct madera *madera)
 */
switch (madera->type) {
case CS47L15:
+   madera->reset_errata = true;
+   break;
case CS47L35:
case CS47L90:
case CS47L91:
@@ -553,13 +562,19 @@ int madera_dev_init(struct madera *madera)
goto err_dcvdd;
}
 
+   if (madera->reset_errata)
+   madera_disable_hard_reset(madera);
+
ret = regulator_enable(madera->dcvdd);
if (ret) {
dev_err(dev, "Failed to enable DCVDD: %d\n", ret);
goto err_enable;
}
 
-   madera_disable_hard_reset(madera);
+   if (madera->reset_errata)
+   usleep_range(ERRATA_DCVDD_MIN_US, ERRATA_DCVDD_MAX_US);
+   else
+   madera_disable_hard_reset(madera);
 
regcache_cache_only(madera->regmap, false);
regcache_cache_only(madera->regmap_32bit, false);
@@ -667,7 +682,7 @@ int madera_dev_init(struct madera *madera)
 * It looks like a device we support. If we don't have a hard reset
 * we can now attempt a soft reset.
 */
-   if (!madera->pdata.reset) {
+   if (!madera->pdata.reset || madera->reset_errata) {
ret = madera_soft_reset(madera);
if (ret)
goto err_reset;
diff --git a/include/linux/mfd/madera/core.h b/include/linux/mfd/madera/core.h
index ad2c138105d4b..03a8a788424a2 100644
--- a/include/linux/mfd/madera/core.h
+++ b/include/linux/mfd/madera/core.h
@@ -186,6 +186,7 @@ struct madera {
struct regulator_bulk_data core_supplies[MADERA_MAX_CORE_SUPPLIES];
struct regulator *dcvdd;
bool internal_dcvdd;
+   bool reset_errata;
 
struct madera_pdata pdata;
 
-- 
2.11.0

Re: [PATCH AUTOSEL 5.8 14/29] regmap: debugfs: Fix handling of name string for debugfs init delays

[Charles Keepax]

On Mon, Sep 28, 2020 at 09:30:11PM -0400, Sasha Levin wrote:
> From: Charles Keepax 
> 
> [ Upstream commit 94cc89eb8fa5039fcb6e3e3d50f929ddcccee095 ]
> 
> In regmap_debugfs_init the initialisation of the debugfs is delayed
> if the root node isn't ready yet. Most callers of regmap_debugfs_init
> pass the name from the regmap_config, which is considered temporary
> ie. may be unallocated after the regmap_init call returns. This leads
> to a potential use after free, where config->name has been freed by
> the time it is used in regmap_debugfs_initcall.
> 

Afraid this patch had some issues if you are back porting it you
definitely need to take these two patches as well:

commit 1d512ee861b80da63cbc501b973c53131aa22f29
regmap: debugfs: Fix more error path regressions

commit d36cb0205f034e943aa29e35b59c6a441f0056b5
regmap: debugfs: Add back in erroneously removed initialisation of ret

Thanks,
Charles

Re: [PATCH 4/8] mfd: wm: Constify static struct resource

[Charles Keepax]

On Tue, Sep 22, 2020 at 09:26:55PM +0200, Rikard Falkeborn wrote:
> Constify a number of static struct resource. The only usage of the
> structs are to assign their address to the resources field in the
> mfd_cell struct. This allows the compiler to put them in read-only
> memory. Done with the help of Coccinelle.
> 
> Signed-off-by: Rikard Falkeborn 
> ---

Acked-by: Charles Keepax 

Thanks,
Charles

Re: [PATCH 41/42] mfd: wm8400: use PLATFORM_DEVID_NONE

[Charles Keepax]

On Mon, Sep 21, 2020 at 10:50:15PM +0200, Krzysztof Kozlowski wrote:
> Use PLATFORM_DEVID_NONE define instead of "-1" value because:
>  - it brings some meaning,
>  - it might point attention why auto device ID was not used.
> 
> Signed-off-by: Krzysztof Kozlowski 
> ---

Acked-by: Charles Keepax 

Thanks,
Charles

Re: [PATCH 42/42] mfd: wm8994: use PLATFORM_DEVID_NONE

[Charles Keepax]

On Mon, Sep 21, 2020 at 10:50:16PM +0200, Krzysztof Kozlowski wrote:
> Use PLATFORM_DEVID_NONE define instead of "-1" value because:
>  - it brings some meaning,
>  - it might point attention why auto device ID was not used.
> 
> Signed-off-by: Krzysztof Kozlowski 
> ---

Acked-by: Charles Keepax 

Thanks,
Charles

Re: [PATCH 01/42] mfd: arizona: use PLATFORM_DEVID_NONE

[Charles Keepax]

On Mon, Sep 21, 2020 at 10:49:35PM +0200, Krzysztof Kozlowski wrote:
> Use PLATFORM_DEVID_NONE define instead of "-1" value because:
>  - it brings some meaning,
>  - it might point attention why auto device ID was not used.
> 
> Signed-off-by: Krzysztof Kozlowski 
> ---

Acked-by: Charles Keepax 

Thanks,
Charles

Re: [PATCH v2 09/13] dt-bindings: pinctrl: include common schema in GPIO controllers

[Charles Keepax]

On Thu, Sep 17, 2020 at 06:52:57PM +0200, Krzysztof Kozlowski wrote:
> Include the common GPIO schema in GPIO controllers to be sure all common
> properties are properly validated.
> 
> Signed-off-by: Krzysztof Kozlowski 
> 
> ---

For the Cirrus bits:

Acked-by: Charles Keepax 

Thanks,
Charles

Re: [PATCH v2 08/13] dt-bindings: mfd: include common schema in GPIO controllers

[Charles Keepax]

On Thu, Sep 17, 2020 at 06:52:56PM +0200, Krzysztof Kozlowski wrote:
> Include the common GPIO schema in GPIO controllers to be sure all common
> properties are properly validated.
> 
> Signed-off-by: Krzysztof Kozlowski 
> 
> ---

For the Cirrus/Wolfson bits:

Acked-by: Charles Keepax 

Thanks,
Charles

[PATCH] regmap: debugfs: Fix more error path regressions

[Charles Keepax]

Many error paths in __regmap_init rely on ret being pre-initialised to
-EINVAL, add an extra initialisation in after the new call to
regmap_set_name.

Fixes: 94cc89eb8fa5 ("regmap: debugfs: Fix handling of name string for debugfs 
init delays")
Reported-by: Dan Carpenter 
Signed-off-by: Charles Keepax 
---
 drivers/base/regmap/regmap.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index b24f14ea96d8a..4c2a114584236 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -723,6 +723,8 @@ struct regmap *__regmap_init(struct device *dev,
if (ret)
goto err_map;
 
+   ret = -EINVAL; /* Later error paths rely on this */
+
if (config->disable_locking) {
map->lock = map->unlock = regmap_lock_unlock_none;
map->can_sleep = config->can_sleep;
-- 
2.11.0

Re: [PATCH] regmap: debugfs: Add back in erroneously removed initialisation of ret

[Charles Keepax]

On Fri, Sep 18, 2020 at 03:38:43PM +0300, Dan Carpenter wrote:
> On Fri, Sep 18, 2020 at 12:20:02PM +0100, Charles Keepax wrote:
> > Fixes: 94cc89eb8fa5 ("regmap: debugfs: Fix handling of name string for 
> > debugfs init delays")
> > Reported-by: kernel test robot 
> > Reported-by: Dan Carpenter 
> > Signed-off-by: Charles Keepax 
> > ---
> >  drivers/base/regmap/regmap.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
> > index d0f7cc574ff34..b24f14ea96d8a 100644
> > --- a/drivers/base/regmap/regmap.c
> > +++ b/drivers/base/regmap/regmap.c
> > @@ -706,9 +706,9 @@ struct regmap *__regmap_init(struct device *dev,
> >  const char *lock_name)
> >  {
> > struct regmap *map;
> > +   int ret = -EINVAL;
> > enum regmap_endian reg_endian, val_endian;
> > int i, j;
> > -   int ret;
> >  
> > if (!config)
> > goto err;
> 
> Hi Charles, this isn't enough.  There are several goto err_map; paths
> were "ret" is zero.  That will result in an Oops in the caller.  It has
> to be an error code.
> 

Aye, sorry thought it was just cause I removed the initialisation
but yeah there is more fall out. Apologies for mucking this up,
third time lucky hopefully.

Thanks,
Charles

[PATCH] regmap: debugfs: Add back in erroneously removed initialisation of ret

[Charles Keepax]

Fixes: 94cc89eb8fa5 ("regmap: debugfs: Fix handling of name string for debugfs 
init delays")
Reported-by: kernel test robot 
Reported-by: Dan Carpenter 
Signed-off-by: Charles Keepax 
---
 drivers/base/regmap/regmap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index d0f7cc574ff34..b24f14ea96d8a 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -706,9 +706,9 @@ struct regmap *__regmap_init(struct device *dev,
 const char *lock_name)
 {
struct regmap *map;
+   int ret = -EINVAL;
enum regmap_endian reg_endian, val_endian;
int i, j;
-   int ret;
 
if (!config)
goto err;
-- 
2.11.0

[PATCH v2] regmap: debugfs: Fix handling of name string for debugfs init delays

[Charles Keepax]

In regmap_debugfs_init the initialisation of the debugfs is delayed
if the root node isn't ready yet. Most callers of regmap_debugfs_init
pass the name from the regmap_config, which is considered temporary
ie. may be unallocated after the regmap_init call returns. This leads
to a potential use after free, where config->name has been freed by
the time it is used in regmap_debugfs_initcall.

This situation can be seen on Zynq, where the architecture init_irq
callback registers a syscon device, using a local variable for the
regmap_config. As init_irq is very early in the platform bring up the
regmap debugfs root isn't ready yet. Although this doesn't crash it
does result in the debugfs entry not having the correct name.

Regmap already sets map->name from config->name on the regmap_init
path and the fact that a separate field is used to pass the name
to regmap_debugfs_init appears to be an artifact of the debugfs
name being added before the map name. As such this patch updates
regmap_debugfs_init to use map->name, which is already duplicated from
the config avoiding the issue.

This does however leave two lose ends, both regmap_attach_dev and
regmap_reinit_cache can be called after a regmap is registered and
would have had the effect of applying a new name to the debugfs
entries. In both of these cases it was chosen to update the map
name. In the case of regmap_attach_dev there are 3 users that
currently use this function to update the name, thus doing so avoids
changes for those users and it seems reasonable that attaching
a device would want to set the name of the map. In the case of
regmap_reinit_cache the primary use-case appears to be devices that
need some register access to identify the device (for example devices
in the same family) and then update the cache to match the exact
hardware. Whilst no users do currently update the name here, given the
use-case it seemed reasonable the name might want to be updated once
the device is better identified.

Signed-off-by: Charles Keepax 
---
 drivers/base/regmap/internal.h   |  4 ++--
 drivers/base/regmap/regmap-debugfs.c |  7 +++---
 drivers/base/regmap/regmap.c | 44 +++-
 3 files changed, 38 insertions(+), 17 deletions(-)

diff --git a/drivers/base/regmap/internal.h b/drivers/base/regmap/internal.h
index 8a59359e145f6..e7ab94cc327a9 100644
--- a/drivers/base/regmap/internal.h
+++ b/drivers/base/regmap/internal.h
@@ -220,7 +220,7 @@ struct regmap_field {
 
 #ifdef CONFIG_DEBUG_FS
 extern void regmap_debugfs_initcall(void);
-extern void regmap_debugfs_init(struct regmap *map, const char *name);
+extern void regmap_debugfs_init(struct regmap *map);
 extern void regmap_debugfs_exit(struct regmap *map);
 
 static inline void regmap_debugfs_disable(struct regmap *map)
@@ -230,7 +230,7 @@ static inline void regmap_debugfs_disable(struct regmap 
*map)
 
 #else
 static inline void regmap_debugfs_initcall(void) { }
-static inline void regmap_debugfs_init(struct regmap *map, const char *name) { 
}
+static inline void regmap_debugfs_init(struct regmap *map) { }
 static inline void regmap_debugfs_exit(struct regmap *map) { }
 static inline void regmap_debugfs_disable(struct regmap *map) { }
 #endif
diff --git a/drivers/base/regmap/regmap-debugfs.c 
b/drivers/base/regmap/regmap-debugfs.c
index f58baff2be0af..b6d63ef16b442 100644
--- a/drivers/base/regmap/regmap-debugfs.c
+++ b/drivers/base/regmap/regmap-debugfs.c
@@ -17,7 +17,6 @@
 
 struct regmap_debugfs_node {
struct regmap *map;
-   const char *name;
struct list_head link;
 };
 
@@ -544,11 +543,12 @@ static const struct file_operations 
regmap_cache_bypass_fops = {
.write = regmap_cache_bypass_write_file,
 };
 
-void regmap_debugfs_init(struct regmap *map, const char *name)
+void regmap_debugfs_init(struct regmap *map)
 {
struct rb_node *next;
struct regmap_range_node *range_node;
const char *devname = "dummy";
+   const char *name = map->name;
 
/*
 * Userspace can initiate reads from the hardware over debugfs.
@@ -569,7 +569,6 @@ void regmap_debugfs_init(struct regmap *map, const char 
*name)
if (!node)
return;
node->map = map;
-   node->name = name;
mutex_lock(_debugfs_early_lock);
list_add(>link, _debugfs_early_list);
mutex_unlock(_debugfs_early_lock);
@@ -679,7 +678,7 @@ void regmap_debugfs_initcall(void)
 
mutex_lock(_debugfs_early_lock);
list_for_each_entry_safe(node, tmp, _debugfs_early_list, link) {
-   regmap_debugfs_init(node->map, node->name);
+   regmap_debugfs_init(node->map);
list_del(>link);
kfree(node);
}
diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index 2807e544658e0..4876e5f543c70 100644
--- a/drivers/base/regm

[PATCH] regmap: debugfs: Duplicate name string if delaying debugfs init

[Charles Keepax]

In regmap_debugfs_init the initialisation of the debugfs is delayed
if the root node isn't ready yet. Most callers of regmap_debugfs_init
pass the name from the regmap_config, which is considered temporary
ie. may be unallocated after the regmap_init call returns. This leads
to a potential use after free, where config->name has been freed by
the time it is used in regmap_debugfs_initcall.

This situation can be seen on Zynq, where the architecture init_irq
callback registers a syscon device, using a local variable for the
regmap_config. As init_irq is very early in the platform bring up the
regmap debugfs root isn't ready yet. Although this doesn't crash it
does result in the debugfs entry not having the correct name.

Signed-off-by: Charles Keepax 
---
 drivers/base/regmap/regmap-debugfs.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/base/regmap/regmap-debugfs.c 
b/drivers/base/regmap/regmap-debugfs.c
index f58baff2be0af..184fc327192bf 100644
--- a/drivers/base/regmap/regmap-debugfs.c
+++ b/drivers/base/regmap/regmap-debugfs.c
@@ -569,7 +569,12 @@ void regmap_debugfs_init(struct regmap *map, const char 
*name)
if (!node)
return;
node->map = map;
-   node->name = name;
+   node->name = kstrdup(name, GFP_KERNEL);
+   if (!node->name) {
+   kfree(node);
+   return;
+   }
+
mutex_lock(_debugfs_early_lock);
list_add(>link, _debugfs_early_list);
mutex_unlock(_debugfs_early_lock);
@@ -681,6 +686,7 @@ void regmap_debugfs_initcall(void)
list_for_each_entry_safe(node, tmp, _debugfs_early_list, link) {
regmap_debugfs_init(node->map, node->name);
list_del(>link);
+   kfree(node->name);
kfree(node);
}
mutex_unlock(_debugfs_early_lock);
-- 
2.11.0

Re: [PATCH] regmap: debugfs: Duplicate name string if delaying debugfs init

[Charles Keepax]

On Wed, Sep 16, 2020 at 05:14:18PM +0100, Mark Brown wrote:
> On Wed, Sep 16, 2020 at 04:44:33PM +0100, Charles Keepax wrote:
> 
> > -   node->name = name;
> > +   node->name = kstrdup(name, GFP_KERNEL);
> > +   if (!node->name) {
> 
> Two things here - one is that this should be kstrdup_const(), the other
> is that we already took a copy of the name in __regmap_init() so the
> thing to do here is to change the regmap_debugfs_init() call there to
> use the copy we just made rather than the copy in the config.

Thanks Mark, I will have a look and update.

Thanks,
Charles

Re: [linux-sunxi] [PATCH 05/16] ASoc: sun4i-i2s: Add 20 and 24 bit support

[Charles Keepax]

On Thu, Sep 03, 2020 at 09:40:23AM +0200, Maxime Ripard wrote:
> On Wed, Sep 02, 2020 at 09:22:33PM -0500, Samuel Holland wrote:
> > On 9/2/20 1:10 PM, Jernej Škrabec wrote:
> > > Hi Samuel!
> > > 
> > > Dne petek, 10. julij 2020 ob 07:44:51 CEST je Samuel Holland napisal(a):
> > >> On 7/4/20 6:38 AM, Clément Péron wrote:
> > >>> From: Marcus Cooper 
> > >>>
> > >>> Extend the functionality of the driver to include support of 20 and
> > >>> 24 bits per sample.
> > >>>
> > >>> Signed-off-by: Marcus Cooper 
> > >>> Signed-off-by: Clément Péron 
> > >>> ---
> > >>>
> > >>>  sound/soc/sunxi/sun4i-i2s.c | 11 +--
> > >>>  1 file changed, 9 insertions(+), 2 deletions(-)
> > >>>
> > >>> diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
> > >>> index f78167e152ce..bc7f9343bc7a 100644
> > >>> --- a/sound/soc/sunxi/sun4i-i2s.c
> > >>> +++ b/sound/soc/sunxi/sun4i-i2s.c
> > >>> @@ -577,6 +577,9 @@ static int sun4i_i2s_hw_params(struct
> > >>> snd_pcm_substream *substream,> 
> > >>> case 16:
> > >>> width = DMA_SLAVE_BUSWIDTH_2_BYTES;
> > >>> break;
> > >>>
> > >>> +   case 32:
> > >>> +   width = DMA_SLAVE_BUSWIDTH_4_BYTES;
> > >>> +   break;
> > >>
> > >> This breaks the sun4i variants, because sun4i_i2s_get_wss returns 4 for 
> > >> a 32
> > >> bit width, but it needs to return 3.
> > > 
> > > I'm not sure what has WSS with physical width and DMA?
> > 
> > This is the change where creating a S24_LE stream no longer fails with 
> > -EINVAL.
> > So this is the change where userspace stops downsampling 24-bit audio 
> > sources.
> > So this is the change where playback of 24-bit audio sources breaks, 
> > because WSS
> > is programmed wrong.
> > 
> > >> As a side note, I wonder why we use the physical width (the spacing 
> > >> between
> > >> samples in RAM) to drive the slot width. S24_LE takes up 4 bytes per 
> > >> sample
> > >> in RAM, which we need for DMA. But I don't see why we would want to
> > >> transmit the padding over the wire. I would expect it to be transmitted 
> > >> the
> > >> same as S24_3LE (which has no padding). It did not matter before, because
> > >> the only supported format had no padding.
> > > 
> > > Allwinner DMA engines support only 1, 2, 4 and sometimes 8 bytes for bus 
> > > width, so if sample is 24 bits in size, we have no other way but to 
> > > transmit 
> > > padding too.
> > 
> > I understand why we do 4 byte DMA from RAM <=> I2S FIFO; that was not my
> > question. I'm referring to the actual wire format (FIFO <=> PCM_DIN/DOUT). 
> > The
> > sample is already truncated from 32 bits to 24 bits in the FIFO -- that's 
> > what
> > TXIM and RXOM in FIFO_CTRL control.
> > 
> > If a sample is 24 bits wide, why would we send 32 BCLKs for every LRCK? I 
> > would
> > expect the slot width to match the sample resolution by default. But yet we 
> > have
> > this code in the driver:
> > 
> > unsigned int word_size = params_width(params);
> > unsigned int slot_width = params_physical_width(params);
> > 
> > I think slot_width should be the same as word_size, and I suggest changing 
> > it
> > before adding 20/24-bit support.
> 
> Generally speaking, the slot width doesn't necessarily match the
> physical width. With TDM for example you may very well have slots
> larger than their samples.
> 
> That being said, S24 is explicitly a format where you send a sample of
> 24 bits in a 32-bit word (in the lowest three bytes, little endian)
> 
> See:
> https://elixir.bootlin.com/linux/v5.9-rc3/source/sound/core/pcm_misc.c#L75
> https://mailman.alsa-project.org/pipermail/alsa-devel/2013-April/061073.html
> 
> 24 bits of data over three bytes like you suggest is S24_3LE
> 

My understanding is physical_width refers to the in memory
representation, but shouldn't be used to control the slot width
on the bus. If not specified otherwise (say through the set_tdm
callback), and if the appropriate BCLK is supported, then the slot
should be just large enough to hold the data.

Thanks,
Charles

[PATCH] regulator: lochnagar: Add additional VDDCORE range

[Charles Keepax]

In the case of an unrecognised mini-card the Lochnagar will not
initialise the VDDCORE voltage register leading to a value outside of the
current range. Add an additional range to cover these values, initially
this wasn't done since they are duplicates of the existing minimum
value.

Signed-off-by: Charles Keepax 
---
 drivers/regulator/lochnagar-regulator.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/regulator/lochnagar-regulator.c 
b/drivers/regulator/lochnagar-regulator.c
index 5ea3e41416849..cb71fa5f43c3e 100644
--- a/drivers/regulator/lochnagar-regulator.c
+++ b/drivers/regulator/lochnagar-regulator.c
@@ -98,6 +98,7 @@ static const struct regulator_ops lochnagar_vddcore_ops = {
 };
 
 static const struct linear_range lochnagar_vddcore_ranges[] = {
+   REGULATOR_LINEAR_RANGE(60, 0,0x7, 0),
REGULATOR_LINEAR_RANGE(60, 0x8, 0x41, 12500),
 };
 
-- 
2.11.0

Re: [PATCH 2/2] ASoC: wm8994: Ensure the device is resumed in wm89xx_mic_detect functions

[Charles Keepax]

On Thu, Aug 27, 2020 at 07:33:57PM +0200, Sylwester Nawrocki wrote:
> When the wm8958_mic_detect, wm8994_mic_detect functions get called from
> the machine driver, e.g. from the card's late_probe() callback, the CODEC
> device may be PM runtime suspended and any regmap writes have no effect.
> Add PM runtime calls to these functions to ensure the device registers
> are updated as expected.
> This suppresses an error during boot
> "wm8994-codec: ASoC: error at snd_soc_component_update_bits on wm8994-codec"
> caused by the regmap access error due to the cache_only flag being set.
> 
> Signed-off-by: Sylwester Nawrocki 
> ---

Acked-by: Charles Keepax 

Thanks,
Charles

Re: [PATCH 1/2] ASoC: wm8994: Skip setting of the WM8994_MICBIAS register for WM1811

[Charles Keepax]

On Thu, Aug 27, 2020 at 07:33:56PM +0200, Sylwester Nawrocki wrote:
> The WM8994_MICBIAS register is not available in the WM1811 CODEC so skip
> initialization of that register for that device.
> This suppresses an error during boot:
> "wm8994-codec: ASoC: error at snd_soc_component_update_bits on wm8994-codec"
> 
> Signed-off-by: Sylwester Nawrocki 
> ---

Acked-by: Charles Keepax 

Thanks,
Charles