Re: [PATCH v1 1/7] soc: imx: gpcv2: check for errors when r/w registers

[Andrey Smirnov]

On Wed, Apr 7, 2021 at 2:21 PM Adrien Grassein
 wrote:
>
> Errors were not checked after each access to registers

FWIW, I didn't write any error checking code on purpose since all of
those are memory mapped registers and I don't think there's a case for
those to error out. Don't have a strong opinion on this though.

> and clocks initialisation.
>
> Signed-off-by: Adrien Grassein 
> ---
>  drivers/soc/imx/gpcv2.c | 62 ++---
>  1 file changed, 45 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c
> index db7e7fc321b1..8ec5b1b817c7 100644
> --- a/drivers/soc/imx/gpcv2.c
> +++ b/drivers/soc/imx/gpcv2.c
> @@ -140,8 +140,12 @@ static int imx_gpc_pu_pgc_sw_pxx_req(struct 
> generic_pm_domain *genpd,
> int i, ret = 0;
> u32 pxx_req;
>
> -   regmap_update_bits(domain->regmap, GPC_PGC_CPU_MAPPING,
> -  domain->bits.map, domain->bits.map);
> +   ret = regmap_update_bits(domain->regmap, GPC_PGC_CPU_MAPPING,
> +domain->bits.map, domain->bits.map);
> +   if (ret) {
> +   dev_err(domain->dev, "failed to map GPC PGC domain\n");
> +   return ret;
> +   }
>
> if (has_regulator && on) {
> ret = regulator_enable(domain->regulator);
> @@ -152,19 +156,39 @@ static int imx_gpc_pu_pgc_sw_pxx_req(struct 
> generic_pm_domain *genpd,
> }
>
> /* Enable reset clocks for all devices in the domain */
> -   for (i = 0; i < domain->num_clks; i++)
> -   clk_prepare_enable(domain->clk[i]);
> +   for (i = 0; i < domain->num_clks; i++) {
> +   ret = clk_prepare_enable(domain->clk[i]);
> +   if (ret) {
> +   dev_err(domain->dev, "failed to enable clocks\n");
> +   goto disable_clocks;
> +   }
> +   }
>
> -   if (enable_power_control)
> -   regmap_update_bits(domain->regmap, GPC_PGC_CTRL(domain->pgc),
> -  GPC_PGC_CTRL_PCR, GPC_PGC_CTRL_PCR);
> +   if (enable_power_control) {
> +   ret = regmap_update_bits(domain->regmap, 
> GPC_PGC_CTRL(domain->pgc),
> +GPC_PGC_CTRL_PCR, GPC_PGC_CTRL_PCR);
> +   if (ret) {
> +   dev_err(domain->dev, "failed to enable power 
> control\n");
> +   goto disable_clocks;
> +   }
> +   }
>
> -   if (domain->bits.hsk)
> -   regmap_update_bits(domain->regmap, GPC_PU_PWRHSK,
> -  domain->bits.hsk, on ? domain->bits.hsk : 
> 0);
> +   if (domain->bits.hsk) {
> +   ret = regmap_update_bits(domain->regmap, GPC_PU_PWRHSK,
> +domain->bits.hsk,
> +on ? domain->bits.hsk : 0);
> +   if (ret) {
> +   dev_err(domain->dev, "Failed to initiate 
> handshake\n");
> +   goto disable_power_control;
> +   }
> +   }
>
> -   regmap_update_bits(domain->regmap, offset,
> -  domain->bits.pxx, domain->bits.pxx);
> +   ret = regmap_update_bits(domain->regmap, offset,
> +domain->bits.pxx, domain->bits.pxx);
> +   if (ret) {
> +   dev_err(domain->dev, "failed to command PGC\n");
> +   goto disable_power_control;
> +   }
>
> /*
>  * As per "5.5.9.4 Example Code 4" in IMX7DRM.pdf wait
> @@ -173,8 +197,15 @@ static int imx_gpc_pu_pgc_sw_pxx_req(struct 
> generic_pm_domain *genpd,
> ret = regmap_read_poll_timeout(domain->regmap, offset, pxx_req,
>!(pxx_req & domain->bits.pxx),
>0, USEC_PER_MSEC);
> -   if (ret) {
> +   if (ret)
> dev_err(domain->dev, "failed to command PGC\n");
> +
> +disable_power_control:
> +   if (enable_power_control)
> +   regmap_update_bits(domain->regmap, GPC_PGC_CTRL(domain->pgc),
> +  GPC_PGC_CTRL_PCR, 0);
> +
> +   if (ret) {
> /*
>  * If we were in a process of enabling a
>  * domain and failed we might as well disable
> @@ -185,10 +216,7 @@ static int imx_gpc_pu_pgc_sw_pxx_req(struct 
> generic_pm_domain *genpd,
> on = !on;
> }
>
> -   if (enable_power_control)
> -   regmap_update_bits(domain->regmap, GPC_PGC_CTRL(domain->pgc),
> -  GPC_PGC_CTRL_PCR, 0);
> -
> +disable_clocks:
> /* Disable reset clocks for all devices in the domain */
> for (i = 0; i < domain->num_clks; i++)
> clk_disable_unprepare(domain->clk[i]);
> --
> 2.25.1
>

Re: pinctrl: sx150x bug

[Andrey Smirnov]

On Thu, Aug 6, 2020 at 2:41 AM Linus Walleij  wrote:
>
> Hi Martin,
>
> thanks for your report, let's check with Peter, Neil and Andrey who also
> use this expander if they also see this problem (CC).
>

Looks reasonable. I haven't used that HW in a while, so I added Chris
Healy to this thread, since he might be in a better position to
comment on this and maybe even provide a Tested-by

> On Wed, Aug 5, 2020 at 11:28 AM Martin DEVERA  wrote:
>
> > I encountered bug in SX1502 expander driver in 5.7.7. Here is relevant
> > DTS part:
> >
> > compatible = "semtech,sx1502q";
> > gpio4_cfg_pins: gpio2-cfg {
> >  pins = "gpio5";
> >  output-high;
> >  };
> >
> > And part of OOPS:
> >
> > [0.673996] [] (gpiochip_get_data) from []
> > (sx150x_gpio_direction_output+0xd)
> > [0.683259] [] (sx150x_gpio_direction_output) from
> > [] (sx150x_pinconf_set+0x)
> > [0.692796] [] (sx150x_pinconf_set) from []
> > (pinconf_apply_setting+0x39/0x7e)
> > [0.701635] [] (pinconf_apply_setting) from []
> > (pinctrl_commit_state+0xa5/0x)
> > [0.710648] [] (pinctrl_commit_state) from []
> > (pinctrl_enable+0xff/0x1d4)
> > [0.719139] [] (pinctrl_enable) from []
> > (sx150x_probe+0x1a3/0x358)
> > [0.727027] [] (sx150x_probe) from []
> > (i2c_device_probe+0x1bb/0x1dc)
> >
> > The problem is that sx150x_pinconf_set uses sx150x_gpio_direction_output
> > but gpio is not
> > setup yet. Patch below fixes it but I'm not sure whether is it correct:
> >
> > diff --git a/drivers/pinctrl/pinctrl-sx150x.c
> > b/drivers/pinctrl/pinctrl-sx150x.c
> > index 6e74bd87d959..3f5651edd336 100644
> > --- a/drivers/pinctrl/pinctrl-sx150x.c
> > +++ b/drivers/pinctrl/pinctrl-sx150x.c
> > @@ -1154,12 +1154,6 @@ static int sx150x_probe(struct i2c_client *client,
> >  return ret;
> >  }
> >
> > -   ret = pinctrl_enable(pctl->pctldev);
> > -   if (ret) {
> > -   dev_err(dev, "Failed to enable pinctrl device\n");
> > -   return ret;
> > -   }
> > -
> >  /* Register GPIO controller */
> >  pctl->gpio.base = -1;
> >  pctl->gpio.ngpio = pctl->data->npins;
> > @@ -1191,6 +1185,12 @@ static int sx150x_probe(struct i2c_client *client,
> >  if (ret)
> >  return ret;
> >
> > +   ret = pinctrl_enable(pctl->pctldev);
> > +   if (ret) {
> > +   dev_err(dev, "Failed to enable pinctrl device\n");
> > +   return ret;
> > +   }
> > +
> >  ret = gpiochip_add_pin_range(>gpio, dev_name(dev),
> >   0, 0, pctl->data->npins);
> >  if (ret)
>
> I don't see any problem with the patch, can you send a proper patch
> with git-send-email so we can test it and apply it if it works for the
> other users? Include the mentioned people on CC.
>
> Yours,
> Linus Walleij

[PATCH] crypto: caam - add clock info for VFxxx SoCs

[Andrey Smirnov]

Add a small bit of plumbing necessary to use CAAM on VFxxx SoCs.

Signed-off-by: Andrey Smirnov 
Cc: Chris Healy 
Cc: Horia Geantă 
Cc: Herbert Xu 
Cc: Fabio Estevam 
Cc: linux-...@nxp.com
Cc: linux-cry...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/crypto/caam/ctrl.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c
index 4fcdd262e581..3d7e87856646 100644
--- a/drivers/crypto/caam/ctrl.c
+++ b/drivers/crypto/caam/ctrl.c
@@ -527,11 +527,21 @@ static const struct caam_imx_data caam_imx6ul_data = {
.num_clks = ARRAY_SIZE(caam_imx6ul_clks),
 };

+static const struct clk_bulk_data caam_vf610_clks[] = {
+   { .id = "ipg" },
+};
+
+static const struct caam_imx_data caam_vf610_data = {
+   .clks = caam_vf610_clks,
+   .num_clks = ARRAY_SIZE(caam_vf610_clks),
+};
+
 static const struct soc_device_attribute caam_imx_soc_table[] = {
{ .soc_id = "i.MX6UL", .data = _imx6ul_data },
{ .soc_id = "i.MX6*",  .data = _imx6_data },
{ .soc_id = "i.MX7*",  .data = _imx7_data },
{ .soc_id = "i.MX8M*", .data = _imx7_data },
+   { .soc_id = "VF*", .data = _vf610_data },
{ .family = "Freescale i.MX" },
{ /* sentinel */ }
 };
--
2.21.3

[PATCH] clk: imx: vf610: add CAAM clock

[Andrey Smirnov]

According to Vybrid Security RM, CCM_CCGR11[CG176] can be used to gate
CAAM ipg clock.

Signed-off-by: Horia Geantă 
Signed-off-by: Andrey Smirnov 
Cc: Chris Healy 
Cc: Shawn Guo 
Cc: Fabio Estevam 
Cc: linux-arm-ker...@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-...@nxp.com
---
 drivers/clk/imx/clk-vf610.c | 1 +
 include/dt-bindings/clock/vf610-clock.h | 3 ++-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/imx/clk-vf610.c b/drivers/clk/imx/clk-vf610.c
index cd04e7dc1878..5129ef8e1d6e 100644
--- a/drivers/clk/imx/clk-vf610.c
+++ b/drivers/clk/imx/clk-vf610.c
@@ -438,6 +438,7 @@ static void __init vf610_clocks_init(struct device_node 
*ccm_node)
clk[VF610_CLK_SNVS] = imx_clk_gate2("snvs-rtc", "ipg_bus", CCM_CCGR6, 
CCM_CCGRx_CGn(7));
clk[VF610_CLK_DAP] = imx_clk_gate("dap", "platform_bus", CCM_CCSR, 24);
clk[VF610_CLK_OCOTP] = imx_clk_gate("ocotp", "ipg_bus", CCM_CCGR6, 
CCM_CCGRx_CGn(5));
+   clk[VF610_CLK_CAAM] = imx_clk_gate2("caam", "ipg_bus", CCM_CCGR11, 
CCM_CCGRx_CGn(0));

imx_check_clocks(clk, ARRAY_SIZE(clk));

diff --git a/include/dt-bindings/clock/vf610-clock.h 
b/include/dt-bindings/clock/vf610-clock.h
index 95394f35a74a..0f2d60e884dc 100644
--- a/include/dt-bindings/clock/vf610-clock.h
+++ b/include/dt-bindings/clock/vf610-clock.h
@@ -195,6 +195,7 @@
 #define VF610_CLK_WKPU 186
 #define VF610_CLK_TCON0187
 #define VF610_CLK_TCON1188
-#define VF610_CLK_END  189
+#define VF610_CLK_CAAM 189
+#define VF610_CLK_END  190

 #endif /* __DT_BINDINGS_CLOCK_VF610_H */
--
2.21.3

Re: [PATCH] crypto: caam - make soc match data optional

[Andrey Smirnov]

On Mon, May 18, 2020 at 6:59 PM Horia Geantă  wrote:
>
> On 5/16/2020 7:23 AM, Andrey Smirnov wrote:
> > Vyrbrid devices don't have any clock that need to be taken care of, so
> > make clock data optional on i.MX.
> >
> Vybrid Security RM states that IPG clock used by CAAM
> can be gated by CCM_CCGR11[CG176].
>

Cool, looks like I missed this when I was looking through RM.

> Clock driver needs to be updated accordingly,
> and so will CAAM driver and DT node.
>
> I don't have a board at hand, so patch below is not tested.
>

I'll take it from here and test/submit appropriate patches. Thanks!

> Horia
>
> -- >8 --
>
> Subject: [PATCH] clk: imx: vf610: add CAAM clock
>
> According to Vybrid Security RM, CCM_CCGR11[CG176] can be used to
> gate CAAM ipg clock.
>
> Signed-off-by: Horia Geantă 
> ---
>  drivers/clk/imx/clk-vf610.c | 2 ++
>  include/dt-bindings/clock/vf610-clock.h | 3 ++-
>  2 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/clk/imx/clk-vf610.c b/drivers/clk/imx/clk-vf610.c
> index cd04e7dc1878..4f3066cf1b89 100644
> --- a/drivers/clk/imx/clk-vf610.c
> +++ b/drivers/clk/imx/clk-vf610.c
> @@ -439,6 +439,8 @@ static void __init vf610_clocks_init(struct device_node 
> *ccm_node)
> clk[VF610_CLK_DAP] = imx_clk_gate("dap", "platform_bus", CCM_CCSR, 
> 24);
> clk[VF610_CLK_OCOTP] = imx_clk_gate("ocotp", "ipg_bus", CCM_CCGR6, 
> CCM_CCGRx_CGn(5));
>
> +   clk[VF610_CLK_CAAM] = imx_clk_gate2("caam", "ipg_bus", CCM_CCGR11, 
> CCM_CCGRx_CGn(0));
> +
> imx_check_clocks(clk, ARRAY_SIZE(clk));
>
> clk_set_parent(clk[VF610_CLK_QSPI0_SEL], clk[VF610_CLK_PLL1_PFD4]);
> diff --git a/include/dt-bindings/clock/vf610-clock.h 
> b/include/dt-bindings/clock/vf610-clock.h
> index 95394f35a74a..0f2d60e884dc 100644
> --- a/include/dt-bindings/clock/vf610-clock.h
> +++ b/include/dt-bindings/clock/vf610-clock.h
> @@ -195,6 +195,7 @@
>  #define VF610_CLK_WKPU 186
>  #define VF610_CLK_TCON0187
>  #define VF610_CLK_TCON1188
> -#define VF610_CLK_END  189
> +#define VF610_CLK_CAAM 189
> +#define VF610_CLK_END  190
>
>  #endif /* __DT_BINDINGS_CLOCK_VF610_H */
> --
> 2.17.1

[PATCH] crypto: caam - make soc match data optional

[Andrey Smirnov]

Vyrbrid devices don't have any clock that need to be taken care of, so
make clock data optional on i.MX.

Signed-off-by: Andrey Smirnov 
Cc: Chris Healy 
Cc: Horia Geantă 
Cc: Herbert Xu 
Cc: Fabio Estevam 
Cc: linux-...@nxp.com
Cc: linux-cry...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/crypto/caam/ctrl.c | 7 +--
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c
index 4fcdd262e581..6aba430793cc 100644
--- a/drivers/crypto/caam/ctrl.c
+++ b/drivers/crypto/caam/ctrl.c
@@ -630,12 +630,7 @@ static int caam_probe(struct platform_device *pdev)
imx_soc_match = soc_device_match(caam_imx_soc_table);
caam_imx = (bool)imx_soc_match;

-   if (imx_soc_match) {
-   if (!imx_soc_match->data) {
-   dev_err(dev, "No clock data provided for i.MX SoC");
-   return -EINVAL;
-   }
-
+   if (imx_soc_match && imx_soc_match->data) {
ret = init_clocks(dev, imx_soc_match->data);
if (ret)
return ret;
--
2.21.3

[PATCH] dt-bindings: serial: lpuart: Drop unsupported RS485 bindings

[Andrey Smirnov]

LPUART driver does not support 'rs485-rts-delay' or
'rs485-rx-during-tx' properties. Remove them.

Signed-off-by: Andrey Smirnov 
Cc: Stefan Agner 
Cc: Chris Healy 
Cc: Greg Kroah-Hartman 
Cc: Jiri Slaby 
Cc: linux-...@nxp.com
Cc: linux-ser...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 Documentation/devicetree/bindings/serial/fsl-lpuart.txt | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/serial/fsl-lpuart.txt 
b/Documentation/devicetree/bindings/serial/fsl-lpuart.txt
index 3495eee81d53..f5f5ab0fd14e 100644
--- a/Documentation/devicetree/bindings/serial/fsl-lpuart.txt
+++ b/Documentation/devicetree/bindings/serial/fsl-lpuart.txt
@@ -21,8 +21,7 @@ Required properties:
 Optional properties:
 - dmas: A list of two dma specifiers, one for each entry in dma-names.
 - dma-names: should contain "tx" and "rx".
-- rs485-rts-delay, rs485-rts-active-low, rs485-rx-during-tx,
-  linux,rs485-enabled-at-boot-time: see rs485.txt
+- rs485-rts-active-low, linux,rs485-enabled-at-boot-time: see rs485.txt
 
 Note: Optional properties for DMA support. Write them both or both not.
 
-- 
2.21.0

[PATCH v2] ARM: imx: Drop imx_anatop_usb_chrg_detect_disable()

[Andrey Smirnov]

With commit b5bbe2235361 ("usb: phy: mxs: Disable external charger
detect in mxs_phy_hw_init()") in tree all of the necessary charger
setup is done by the USB PHY driver which covers all of the affected
i.MX6 SoCs.

NOTE: imx_anatop_usb_chrg_detect_disable() was also called for i.MX7D,
but looking at its datasheet it appears to have a different USB PHY IP
block, so executing i.MX6 charger disable configuration seems
unnecessary.

Signed-off-by: Andrey Smirnov 
Cc: Chris Healy 
Cc: Shawn Guo 
Cc: Fabio Estevam 
Cc: Peter Chen 
Cc: linux-...@nxp.com
Cc: linux-arm-ker...@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
---

Changes since [v1]:

- Scope of the patch reduced to remove only
  imx_anatop_usb_chrg_detect_disable() instead of
  imx_anatop_init()

[v1] lore.kernel.org/lkml/20190731180131.8597-1-andrew.smir...@gmail.com

 arch/arm/mach-imx/anatop.c | 20 +---
 1 file changed, 1 insertion(+), 19 deletions(-)

diff --git a/arch/arm/mach-imx/anatop.c b/arch/arm/mach-imx/anatop.c
index 777d8c255501..8fb68c0ec34c 100644
--- a/arch/arm/mach-imx/anatop.c
+++ b/arch/arm/mach-imx/anatop.c
@@ -19,8 +19,6 @@
 #define ANADIG_REG_2P5 0x130
 #define ANADIG_REG_CORE0x140
 #define ANADIG_ANA_MISC0   0x150
-#define ANADIG_USB1_CHRG_DETECT0x1b0
-#define ANADIG_USB2_CHRG_DETECT0x210
 #define ANADIG_DIGPROG 0x260
 #define ANADIG_DIGPROG_IMX6SL  0x280
 #define ANADIG_DIGPROG_IMX7D   0x800
@@ -33,8 +31,6 @@
 #define BM_ANADIG_ANA_MISC0_STOP_MODE_CONFIG   0x1000
 /* Below MISC0_DISCON_HIGH_SNVS is only for i.MX6SL */
 #define BM_ANADIG_ANA_MISC0_DISCON_HIGH_SNVS   0x2000
-#define BM_ANADIG_USB_CHRG_DETECT_CHK_CHRG_B   0x8
-#define BM_ANADIG_USB_CHRG_DETECT_EN_B 0x10
 
 static struct regmap *anatop;
 
@@ -96,16 +92,6 @@ void imx_anatop_post_resume(void)
 
 }
 
-static void imx_anatop_usb_chrg_detect_disable(void)
-{
-   regmap_write(anatop, ANADIG_USB1_CHRG_DETECT,
-   BM_ANADIG_USB_CHRG_DETECT_EN_B
-   | BM_ANADIG_USB_CHRG_DETECT_CHK_CHRG_B);
-   regmap_write(anatop, ANADIG_USB2_CHRG_DETECT,
-   BM_ANADIG_USB_CHRG_DETECT_EN_B |
-   BM_ANADIG_USB_CHRG_DETECT_CHK_CHRG_B);
-}
-
 void __init imx_init_revision_from_anatop(void)
 {
struct device_node *np;
@@ -171,10 +157,6 @@ void __init imx_init_revision_from_anatop(void)
 void __init imx_anatop_init(void)
 {
anatop = syscon_regmap_lookup_by_compatible("fsl,imx6q-anatop");
-   if (IS_ERR(anatop)) {
+   if (IS_ERR(anatop))
pr_err("%s: failed to find imx6q-anatop regmap!\n", __func__);
-   return;
-   }
-
-   imx_anatop_usb_chrg_detect_disable();
 }
-- 
2.21.0

[PATCH 3/3] ARM: dts: imx6qdl-zii-rdu2: Specify supplies for accelerometer

[Andrey Smirnov]

Specify 'vdd' and 'vddio' supplies for accelerometer to avoid warnings
during boot.

Signed-off-by: Andrey Smirnov 
Cc: Fabio Estevam 
Cc: Chris Healy 
Cc: Lucas Stach 
Cc: Shawn Guo 
Cc: linux-arm-ker...@lists.infradead.org,
Cc: linux-kernel@vger.kernel.org
---
 arch/arm/boot/dts/imx6qdl-zii-rdu2.dtsi | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/boot/dts/imx6qdl-zii-rdu2.dtsi 
b/arch/arm/boot/dts/imx6qdl-zii-rdu2.dtsi
index a8c86e621b49..42c0a728216d 100644
--- a/arch/arm/boot/dts/imx6qdl-zii-rdu2.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-zii-rdu2.dtsi
@@ -360,6 +360,8 @@
interrupt-parent = <>;
interrupt-names = "INT2";
interrupts = <20 IRQ_TYPE_LEVEL_LOW>;
+   vdd-supply = <_3p3v>;
+   vddio-supply = <_3p3v>;
};
 
hpa2: amp@60 {
-- 
2.21.0

[PATCH 2/3] ARM: dts: imx6qdl-zii-rdu2: Fix accelerometer interrupt-names

[Andrey Smirnov]

According to Documentation/devicetree/bindings/iio/accel/mma8452.txt,
the correct interrupt-names are "INT1" and "INT2", so fix them
accordingly.

While at it, modify the node to only specify "INT2" since providing
two interrupts is not necessary or useful (the driver will only use
one).

Signed-off-by: Fabio Estevam 
[andrew.smir...@gmail.com modified the patch to drop INT1]
Signed-off-by: Andrey Smirnov 
Cc: Fabio Estevam 
Cc: Chris Healy 
Cc: Lucas Stach 
Cc: Shawn Guo 
Cc: linux-arm-ker...@lists.infradead.org,
Cc: linux-kernel@vger.kernel.org
---

Original patch from Fabio can be seen here:

https://lore.kernel.org/linux-arm-kernel/20191010125300.2822-1-feste...@gmail.com/

 arch/arm/boot/dts/imx6qdl-zii-rdu2.dtsi | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/arch/arm/boot/dts/imx6qdl-zii-rdu2.dtsi 
b/arch/arm/boot/dts/imx6qdl-zii-rdu2.dtsi
index 8d46f7b2722b..a8c86e621b49 100644
--- a/arch/arm/boot/dts/imx6qdl-zii-rdu2.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-zii-rdu2.dtsi
@@ -358,8 +358,8 @@
compatible = "fsl,mma8451";
reg = <0x1c>;
interrupt-parent = <>;
-   interrupt-names = "int1", "int2";
-   interrupts = <18 IRQ_TYPE_LEVEL_LOW>, <20 IRQ_TYPE_LEVEL_LOW>;
+   interrupt-names = "INT2";
+   interrupts = <20 IRQ_TYPE_LEVEL_LOW>;
};
 
hpa2: amp@60 {
@@ -849,7 +849,6 @@
  {
pinctrl_accel: accelgrp {
fsl,pins = <
-   MX6QDL_PAD_SD1_CMD__GPIO1_IO18  0x4001b000
MX6QDL_PAD_SD1_CLK__GPIO1_IO20  0x4001b000
>;
};
-- 
2.21.0

[PATCH 1/3] ARM: dts: imx6qdl-zii-rdu2: Drop GPIO_ACTIVE_LOW form reg_5p0v_user_usb

[Andrey Smirnov]

Drop GPIO_ACTIVE_LOW form reg_5p0v_user_usb since it is ignored by the
gpiolib and results in a warning.

Signed-off-by: Andrey Smirnov 
Cc: Fabio Estevam 
Cc: Chris Healy 
Cc: Lucas Stach 
Cc: Shawn Guo 
Cc: linux-arm-ker...@lists.infradead.org,
Cc: linux-kernel@vger.kernel.org
---
 arch/arm/boot/dts/imx6qdl-zii-rdu2.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/imx6qdl-zii-rdu2.dtsi 
b/arch/arm/boot/dts/imx6qdl-zii-rdu2.dtsi
index 93be00a60c88..8d46f7b2722b 100644
--- a/arch/arm/boot/dts/imx6qdl-zii-rdu2.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-zii-rdu2.dtsi
@@ -68,7 +68,7 @@
regulator-name = "5V_USER_USB";
regulator-min-microvolt = <500>;
regulator-max-microvolt = <500>;
-   gpio = < 22 GPIO_ACTIVE_LOW>;
+   gpio = < 22 0>;
startup-delay-us = <1000>;
};
 
-- 
2.21.0

[PATCH] dt-bindings: mma8452: Re-word 'interrupt-names' description

[Andrey Smirnov]

Current wording in the binding documentation doesn't make it 100%
clear that only one of "INT1" and "INT2" will ever be used by the
driver and that specifying both has no advantages. Re-word it to make
this aspect a bit more explicit.

Signed-off-by: Andrey Smirnov 
Cc: Chris Healy 
Cc: Jonathan Cameron 
Cc: Hartmut Knaack 
Cc: Lars-Peter Clausen 
Cc: Peter Meerwald-Stadler 
Cc: linux-...@vger.kernel.org
Cc: devicet...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 Documentation/devicetree/bindings/iio/accel/mma8452.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/iio/accel/mma8452.txt 
b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
index e132394375a1..b27b6bee9eb6 100644
--- a/Documentation/devicetree/bindings/iio/accel/mma8452.txt
+++ b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
@@ -17,7 +17,7 @@ Optional properties:
 
   - interrupts: interrupt mapping for GPIO IRQ
 
-  - interrupt-names: should contain "INT1" and/or "INT2", the accelerometer's
+  - interrupt-names: should contain "INT1" or "INT2", the accelerometer's
 interrupt line in use.
 
   - vdd-supply: phandle to the regulator that provides vdd power to the 
accelerometer.
-- 
2.21.0

[PATCH v3 0/3] Logitech G920 fixes

[Andrey Smirnov]

Everyone:

This series contains patches to fix a couple of regressions in G920
wheel support by hid-logitech-hidpp driver. Without the patches the
wheel remains stuck in autocentering mode ("resisting" any attempt to
trun) as well as missing support for any FF action.

Thanks,
Andrey Smirnov

Changes since [v2]:

 - Fixes a buggy validity check "HID: logitech-hidpp: rework
   device validation" as pointed out by Benjamin Tissoires

 - Marked "HID: logitech-hidpp: do all FF cleanup in
   hidpp_ff_destroy()" as 5.2+ for stable

Changes since [v1]:

 - "HID: logitech-hidpp: split g920_get_config()" is changed to
   not rely on devres and be a self contained patch

 - Quirk driven behaviour of "HID: logitech-hidpp: add G920 device
   validation quirk" is replaced with generic validation algorithm
   of "HID: logitech-hidpp: rework device validation"

 - Fix for a poteintial race condition is added in
   "HID: logitech-hidpp: do all FF cleanup in hidpp_ff_destroy()"
   as per suggestion by Benjamin Tissoires

 - Collected Tested-by from Sam Bazely for "HID: logitech-hidpp:
   split g920_get_config()" since that patch didn't change
   significantly since [v1]

 - Specified stable kernel versions I think the patches should
   apply to (hopefully I got that right)

[v2] lore.kernel.org/lkml/20191016182935.5616-1-andrew.smir...@gmail.com
[v1] lore.kernel.org/lkml/20191007051240.4410-1-andrew.smir...@gmail.com

Andrey Smirnov (3):
  HID: logitech-hidpp: split g920_get_config()
  HID: logitech-hidpp: rework device validation
  HID: logitech-hidpp: do all FF cleanup in hidpp_ff_destroy()

 drivers/hid/hid-logitech-hidpp.c | 237 +--
 1 file changed, 131 insertions(+), 106 deletions(-)

-- 
2.21.0

[PATCH v3 2/3] HID: logitech-hidpp: rework device validation

[Andrey Smirnov]

G920 device only advertises REPORT_ID_HIDPP_LONG and
REPORT_ID_HIDPP_VERY_LONG in its HID report descriptor, so querying
for REPORT_ID_HIDPP_SHORT with optional=false will always fail and
prevent G920 to be recognized as a valid HID++ device.

To fix this and improve some other aspects, modify
hidpp_validate_device() as follows:

  - Inline the code of hidpp_validate_report() to simplify
distingushing between non-present and invalid report descriptors

  - Drop the check for id >= HID_MAX_IDS || id < 0 since all of our
IDs are static and known to satisfy that at compile time

  - Change the algorithms to check all possible report
types (including very long report) and deem the device as a valid
HID++ device if it supports at least one

  - Treat invalid report length as a hard stop for the validation
algorithm, meaning that if any of the supported reports has
invalid length we assume the worst and treat the device as a
generic HID device.

  - Fold initialization of hidpp->very_long_report_length into
hidpp_validate_device() since it already fetches very long report
length and validates its value

Fixes: fe3ee1ec007b ("HID: logitech-hidpp: allow non HID++ devices to be 
handled by this module")
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=204191
Reported-by: Sam Bazely 
Signed-off-by: Andrey Smirnov 
Cc: Jiri Kosina 
Cc: Benjamin Tissoires 
Cc: Henrik Rydberg 
Cc: Pierre-Loup A. Griffais 
Cc: Austin Palmer 
Cc: linux-in...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: sta...@vger.kernel.org # 5.2+
---
 drivers/hid/hid-logitech-hidpp.c | 54 ++--
 1 file changed, 30 insertions(+), 24 deletions(-)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 85911586b3b6..6e669eb7dc69 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -3498,34 +3498,45 @@ static int hidpp_get_report_length(struct hid_device 
*hdev, int id)
return report->field[0]->report_count + 1;
 }
 
-static bool hidpp_validate_report(struct hid_device *hdev, int id,
- int expected_length, bool optional)
+static bool hidpp_validate_device(struct hid_device *hdev)
 {
-   int report_length;
+   struct hidpp_device *hidpp = hid_get_drvdata(hdev);
+   int id, report_length, supported_reports = 0;
+
+   id = REPORT_ID_HIDPP_SHORT;
+   report_length = hidpp_get_report_length(hdev, id);
+   if (report_length) {
+   if (report_length < HIDPP_REPORT_SHORT_LENGTH)
+   goto bad_device;
 
-   if (id >= HID_MAX_IDS || id < 0) {
-   hid_err(hdev, "invalid HID report id %u\n", id);
-   return false;
+   supported_reports++;
}
 
+   id = REPORT_ID_HIDPP_LONG;
report_length = hidpp_get_report_length(hdev, id);
-   if (!report_length)
-   return optional;
+   if (report_length) {
+   if (report_length < HIDPP_REPORT_LONG_LENGTH)
+   goto bad_device;
 
-   if (report_length < expected_length) {
-   hid_warn(hdev, "not enough values in hidpp report %d\n", id);
-   return false;
+   supported_reports++;
}
 
-   return true;
-}
+   id = REPORT_ID_HIDPP_VERY_LONG;
+   report_length = hidpp_get_report_length(hdev, id);
+   if (report_length) {
+   if (report_length < HIDPP_REPORT_LONG_LENGTH ||
+   report_length > HIDPP_REPORT_VERY_LONG_MAX_LENGTH)
+   goto bad_device;
 
-static bool hidpp_validate_device(struct hid_device *hdev)
-{
-   return hidpp_validate_report(hdev, REPORT_ID_HIDPP_SHORT,
-HIDPP_REPORT_SHORT_LENGTH, false) &&
-  hidpp_validate_report(hdev, REPORT_ID_HIDPP_LONG,
-HIDPP_REPORT_LONG_LENGTH, true);
+   supported_reports++;
+   hidpp->very_long_report_length = report_length;
+   }
+
+   return supported_reports;
+
+bad_device:
+   hid_warn(hdev, "not enough values in hidpp report %d\n", id);
+   return false;
 }
 
 static bool hidpp_application_equals(struct hid_device *hdev,
@@ -3572,11 +3583,6 @@ static int hidpp_probe(struct hid_device *hdev, const 
struct hid_device_id *id)
return hid_hw_start(hdev, HID_CONNECT_DEFAULT);
}
 
-   hidpp->very_long_report_length =
-   hidpp_get_report_length(hdev, REPORT_ID_HIDPP_VERY_LONG);
-   if (hidpp->very_long_report_length > HIDPP_REPORT_VERY_LONG_MAX_LENGTH)
-   hidpp->very_long_report_length = 
HIDPP_REPORT_VERY_LONG_MAX_LENGTH;
-
if (id->group == HID_GROUP_LOGITECH_DJ_DEVICE)
hidpp->quirks |= HIDPP_QUIRK_UNIFYING;
 
-- 
2.21.0

[PATCH v3 1/3] HID: logitech-hidpp: split g920_get_config()

[Andrey Smirnov]

9k_hw 
coretemp snd_hda_codec_hdmi cordic kvm_intel snd_hda_codec_cirrus mac80211 
snd_hda_codec_generic ledtrig_audio kvm snd_hda_intel snd_intel_nhlt irqbypass 
snd_hda_codec btusb btrtl snd_hda_core ath btbcm ssb snd_hwdep btintel snd_seq 
crct10dif_pclmul iTCO_wdt snd_seq_device crc32_pclmul bluetooth mmc_core 
iTCO_vendor_support joydev cfg80211
[   88.320602]  applesmc ghash_clmulni_intel ecdh_generic snd_pcm input_polldev 
intel_cstate ecc intel_uncore thunderbolt snd_timer i2c_i801 libarc4 rfkill 
intel_rapl_perf lpc_ich mei_me pcspkr bcm5974 snd bcma mei soundcore acpi_als 
sbs kfifo_buf sbshc industrialio apple_bl i915 i2c_algo_bit drm_kms_helper drm 
uas crc32c_intel usb_storage video hid_apple
[   88.320630] CR2: 0018
[   88.320633] ---[ end trace 933491c8a4fadeb7 ]---
[   88.320642] RIP: 0010:hidpp_probe+0x61f/0x948 [hid_logitech_hidpp]
[   88.320645] Code: 81 00 00 48 89 ef e8 f0 d6 ff ff 41 89 c6 85 c0 75 b5 0f 
b6 44 24 28 48 8b 5d 00 88 44 24 1e 89 44 24 0c 48 8b 83 18 1c 00 00 <48> 8b 48 
18 48 8b 83 10 19 00 00 48 8b 40 40 48 89 0c 24 0f b7 80
[   88.320647] RSP: 0018:b0a6824aba68 EFLAGS: 00010246
[   88.320650] RAX:  RBX: 93a50756e000 RCX: 00010408
[   88.320652] RDX:  RSI: 93a51f0ad0a0 RDI: 0002d0a0
[   88.320655] RBP: 93a50416da28 R08: 93a50416da70 R09: 93a50416da70
[   88.320657] R10: 00148ae9e60c R11: 000f1525 R12: 93a50756e000
[   88.320659] R13: 93a50756f8d0 R14:  R15: 93a50756fc38
[   88.320662] FS:  7f8d8c1e0940() GS:93a51f08() 
knlGS:
[   88.320664] CS:  0010 DS:  ES:  CR0: 80050033
[   88.320667] CR2: 0018 CR3: 0003996d8003 CR4: 001606e0

To solve this issue:

   1. Split g920_get_config() such that all of the device specific
  communication remains a part of the function and input subsystem
  initialization bits go to hidpp_ff_init()

   2. Move call to hidpp_ff_init() from being a part of
  g920_get_config() to be the last step of .probe(), right after a
  call to hid_hw_start() with connect_mask containing
  HID_CONNECT_HIDINPUT.

Fixes: 91cf9a98ae41 ("HID: logitech-hidpp: make .probe usbhid capable")
Signed-off-by: Andrey Smirnov 
Tested-by: Sam Bazley 
Cc: Jiri Kosina 
Cc: Benjamin Tissoires 
Cc: Henrik Rydberg 
Cc: Pierre-Loup A. Griffais 
Cc: Austin Palmer 
Cc: linux-in...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: sta...@vger.kernel.org # 5.2+
---
 drivers/hid/hid-logitech-hidpp.c | 150 ---
 1 file changed, 96 insertions(+), 54 deletions(-)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 1ac1ecc1e67c..85911586b3b6 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -1669,6 +1669,7 @@ static void hidpp_touchpad_raw_xy_event(struct 
hidpp_device *hidpp_dev,
 
 #define HIDPP_FF_EFFECTID_NONE -1
 #define HIDPP_FF_EFFECTID_AUTOCENTER   -2
+#define HIDPP_AUTOCENTER_PARAMS_LENGTH 18
 
 #define HIDPP_FF_MAX_PARAMS20
 #define HIDPP_FF_RESERVED_SLOTS1
@@ -2009,7 +2010,7 @@ static int hidpp_ff_erase_effect(struct input_dev *dev, 
int effect_id)
 static void hidpp_ff_set_autocenter(struct input_dev *dev, u16 magnitude)
 {
struct hidpp_ff_private_data *data = dev->ff->private;
-   u8 params[18];
+   u8 params[HIDPP_AUTOCENTER_PARAMS_LENGTH];
 
dbg_hid("Setting autocenter to %d.\n", magnitude);
 
@@ -2081,7 +2082,8 @@ static void hidpp_ff_destroy(struct ff_device *ff)
kfree(data->effect_ids);
 }
 
-static int hidpp_ff_init(struct hidpp_device *hidpp, u8 feature_index)
+static int hidpp_ff_init(struct hidpp_device *hidpp,
+struct hidpp_ff_private_data *data)
 {
struct hid_device *hid = hidpp->hid_dev;
struct hid_input *hidinput;
@@ -2089,9 +2091,7 @@ static int hidpp_ff_init(struct hidpp_device *hidpp, u8 
feature_index)
const struct usb_device_descriptor *udesc = 
&(hid_to_usb_dev(hid)->descriptor);
const u16 bcdDevice = le16_to_cpu(udesc->bcdDevice);
struct ff_device *ff;
-   struct hidpp_report response;
-   struct hidpp_ff_private_data *data;
-   int error, j, num_slots;
+   int error, j, num_slots = data->num_effects;
u8 version;
 
if (list_empty(>inputs)) {
@@ -2116,27 +2116,17 @@ static int hidpp_ff_init(struct hidpp_device *hidpp, u8 
feature_index)
for (j = 0; hidpp_ff_effects_v2[j] >= 0; j++)
set_bit(hidpp_ff_effects_v2[j], dev->ffbit);
 
-   /* Read number of slots available in device */
-   error = hidpp_send_fap_command_sync(hidpp, feature_index,
-   HIDPP_FF_GET_INFO, NULL, 0, );
-   if (error) {
-   if (error < 0)
-   return error;

Re: [PATCH v2 3/3] HID: logitech-hidpp: do all FF cleanup in hidpp_ff_destroy()

[Andrey Smirnov]

On Thu, Oct 17, 2019 at 7:31 AM Sasha Levin  wrote:
>
> Hi,
>
> [This is an automated email]
>
> This commit has been processed because it contains a "Fixes:" tag,
> fixing commit: ff21a635dd1a9 HID: logitech-hidpp: Force feedback support for 
> the Logitech G920.
>
> The bot has tested the following trees: v5.3.6, v4.19.79, v4.14.149, v4.9.196.
>
> v5.3.6: Build OK!
> v4.19.79: Failed to apply! Possible dependencies:
> 91cf9a98ae412 ("HID: logitech-hidpp: make .probe usbhid capable")
>
> v4.14.149: Failed to apply! Possible dependencies:
> 91cf9a98ae412 ("HID: logitech-hidpp: make .probe usbhid capable")
>
> v4.9.196: Failed to apply! Possible dependencies:
> 6bd4e65d521f9 ("HID: logitech-hidpp: remove HIDPP_QUIRK_CONNECT_EVENTS")
> 91cf9a98ae412 ("HID: logitech-hidpp: make .probe usbhid capable")
> a4bf6153b3177 ("HID: logitech-hidpp: add a sysfs file to tell we support 
> power_supply")
>
>
> NOTE: The patch will not be queued to stable trees until it is upstream.
>
> How should we proceed with this patch?

Looks like I'd have to mark this one as 5.2+ as well. Please disregard
this series in favor of v3 that will be sent out shortly.

Thanks,
Andrey Smirnov

[PATCH v3 3/3] HID: logitech-hidpp: do all FF cleanup in hidpp_ff_destroy()

[Andrey Smirnov]

All of the FF-related resources belong to corresponding FF device, so
they should be freed as a part of hidpp_ff_destroy() to avoid
potential race condidions.

Fixes: ff21a635dd1a ("HID: logitech-hidpp: Force feedback support for the 
Logitech G920")
Suggested-by: Benjamin Tissoires 
Signed-off-by: Andrey Smirnov 
Cc: Jiri Kosina 
Cc: Benjamin Tissoires 
Cc: Henrik Rydberg 
Cc: Pierre-Loup A. Griffais 
Cc: Austin Palmer 
Cc: linux-in...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: sta...@vger.kernel.org # 5.2+
---
 drivers/hid/hid-logitech-hidpp.c | 33 +---
 1 file changed, 5 insertions(+), 28 deletions(-)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 6e669eb7dc69..8e91e2f06cb4 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -2078,7 +2078,12 @@ static DEVICE_ATTR(range, S_IRUSR | S_IWUSR | S_IRGRP | 
S_IWGRP | S_IROTH, hidpp
 static void hidpp_ff_destroy(struct ff_device *ff)
 {
struct hidpp_ff_private_data *data = ff->private;
+   struct hid_device *hid = data->hidpp->hid_dev;
 
+   hid_info(hid, "Unloading HID++ force feedback.\n");
+
+   device_remove_file(>dev, _attr_range);
+   destroy_workqueue(data->wq);
kfree(data->effect_ids);
 }
 
@@ -2170,31 +2175,6 @@ static int hidpp_ff_init(struct hidpp_device *hidpp,
return 0;
 }
 
-static int hidpp_ff_deinit(struct hid_device *hid)
-{
-   struct hid_input *hidinput = list_entry(hid->inputs.next, struct 
hid_input, list);
-   struct input_dev *dev = hidinput->input;
-   struct hidpp_ff_private_data *data;
-
-   if (!dev) {
-   hid_err(hid, "Struct input_dev not found!\n");
-   return -EINVAL;
-   }
-
-   hid_info(hid, "Unloading HID++ force feedback.\n");
-   data = dev->ff->private;
-   if (!data) {
-   hid_err(hid, "Private data not found!\n");
-   return -EINVAL;
-   }
-
-   destroy_workqueue(data->wq);
-   device_remove_file(>dev, _attr_range);
-
-   return 0;
-}
-
-
 /* ** 
*/
 /*
*/
 /* Device Support 
*/
@@ -3713,9 +3693,6 @@ static void hidpp_remove(struct hid_device *hdev)
 
sysfs_remove_group(>dev.kobj, _attribute_group);
 
-   if (hidpp->quirks & HIDPP_QUIRK_CLASS_G920)
-   hidpp_ff_deinit(hdev);
-
hid_hw_stop(hdev);
cancel_work_sync(>work);
mutex_destroy(>send_mutex);
-- 
2.21.0

Re: [PATCH v2 2/3] HID: logitech-hidpp: rework device validation

[Andrey Smirnov]

On Wed, Oct 16, 2019 at 12:24 PM Benjamin Tissoires
 wrote:
>
> Hi Andrey,
>
> On Wed, Oct 16, 2019 at 8:30 PM Andrey Smirnov  
> wrote:
> >
> > G920 device only advertises REPORT_ID_HIDPP_LONG and
> > REPORT_ID_HIDPP_VERY_LONG in its HID report descriptor, so querying
> > for REPORT_ID_HIDPP_SHORT with optional=false will always fail and
> > prevent G920 to be recognized as a valid HID++ device.
> >
> > To fix this and improve some other aspects, modify
> > hidpp_validate_device() as follows:
> >
> >   - Inline the code of hidpp_validate_report() to simplify
> > distingushing between non-present and invalid report descriptors
> >
> >   - Drop the check for id >= HID_MAX_IDS || id < 0 since all of our
> > IDs are static and known to satisfy that at compile time
> >
> >   - Change the algorithms to check all possible report
> > types (including very long report) and deem the device as a valid
> > HID++ device if it supports at least one
> >
> >   - Treat invalid report length as a hard stop for the validation
> > algorithm, meaning that if any of the supported reports has
> > invalid length we assume the worst and treat the device as a
> > generic HID device.
> >
> >   - Fold initialization of hidpp->very_long_report_length into
> > hidpp_validate_device() since it already fetches very long report
> > length and validates its value
> >
> > Fixes: fe3ee1ec007b ("HID: logitech-hidpp: allow non HID++ devices to be 
> > handled by this module")
> > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=204191
> > Reported-by: Sam Bazely 
> > Signed-off-by: Andrey Smirnov 
> > Cc: Jiri Kosina 
> > Cc: Benjamin Tissoires 
> > Cc: Henrik Rydberg 
> > Cc: Pierre-Loup A. Griffais 
> > Cc: Austin Palmer 
> > Cc: linux-in...@vger.kernel.org
> > Cc: linux-kernel@vger.kernel.org
> > Cc: sta...@vger.kernel.org # 5.2+
> > ---
> >  drivers/hid/hid-logitech-hidpp.c | 54 ++--
> >  1 file changed, 30 insertions(+), 24 deletions(-)
> >
> > diff --git a/drivers/hid/hid-logitech-hidpp.c 
> > b/drivers/hid/hid-logitech-hidpp.c
> > index 85911586b3b6..8c4be991f387 100644
> > --- a/drivers/hid/hid-logitech-hidpp.c
> > +++ b/drivers/hid/hid-logitech-hidpp.c
> > @@ -3498,34 +3498,45 @@ static int hidpp_get_report_length(struct 
> > hid_device *hdev, int id)
> > return report->field[0]->report_count + 1;
> >  }
> >
> > -static bool hidpp_validate_report(struct hid_device *hdev, int id,
> > - int expected_length, bool optional)
> > +static bool hidpp_validate_device(struct hid_device *hdev)
> >  {
> > -   int report_length;
> > +   struct hidpp_device *hidpp = hid_get_drvdata(hdev);
> > +   int id, report_length, supported_reports = 0;
> > +
> > +   id = REPORT_ID_HIDPP_SHORT;
> > +   report_length = hidpp_get_report_length(hdev, id);
> > +   if (report_length) {
> > +   if (report_length < HIDPP_REPORT_SHORT_LENGTH)
> > +   goto bad_device;
> >
> > -   if (id >= HID_MAX_IDS || id < 0) {
> > -   hid_err(hdev, "invalid HID report id %u\n", id);
> > -   return false;
> > +   supported_reports++;
> > }
> >
> > +   id = REPORT_ID_HIDPP_LONG;
> > report_length = hidpp_get_report_length(hdev, id);
> > -   if (!report_length)
> > -   return optional;
> > +   if (report_length) {
> > +   if (report_length < HIDPP_REPORT_LONG_LENGTH)
> > +   goto bad_device;
> >
> > -   if (report_length < expected_length) {
> > -   hid_warn(hdev, "not enough values in hidpp report %d\n", 
> > id);
> > -   return false;
> > +   supported_reports++;
> > }
> >
> > -   return true;
> > -}
> > +   id = REPORT_ID_HIDPP_VERY_LONG;
> > +   report_length = hidpp_get_report_length(hdev, id);
> > +   if (report_length) {
> > +   if (report_length > HIDPP_REPORT_LONG_LENGTH &&
> > +   report_length < HIDPP_REPORT_VERY_LONG_MAX_LENGTH)
>
> Can you double check the conditions here?
> It's late, but I think you inverted the tests as we expect the report
> length to be between HIDPP_REPORT_LONG_LENGTH and
> HIDPP_REPORT_VERY_LONG_MAX_LENGTH inclusive, while here this creates a
> bad_device.

Hmm, I think you are right. Not sure why I didn't catch it on G920
since it support very long reports AFAIR. Will re-spin and double
check in v3. Sorry about that.

Thanks,
Andrey Smirnov

[PATCH v2 2/3] HID: logitech-hidpp: rework device validation

[Andrey Smirnov]

G920 device only advertises REPORT_ID_HIDPP_LONG and
REPORT_ID_HIDPP_VERY_LONG in its HID report descriptor, so querying
for REPORT_ID_HIDPP_SHORT with optional=false will always fail and
prevent G920 to be recognized as a valid HID++ device.

To fix this and improve some other aspects, modify
hidpp_validate_device() as follows:

  - Inline the code of hidpp_validate_report() to simplify
distingushing between non-present and invalid report descriptors

  - Drop the check for id >= HID_MAX_IDS || id < 0 since all of our
IDs are static and known to satisfy that at compile time

  - Change the algorithms to check all possible report
types (including very long report) and deem the device as a valid
HID++ device if it supports at least one

  - Treat invalid report length as a hard stop for the validation
algorithm, meaning that if any of the supported reports has
invalid length we assume the worst and treat the device as a
generic HID device.

  - Fold initialization of hidpp->very_long_report_length into
hidpp_validate_device() since it already fetches very long report
length and validates its value

Fixes: fe3ee1ec007b ("HID: logitech-hidpp: allow non HID++ devices to be 
handled by this module")
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=204191
Reported-by: Sam Bazely 
Signed-off-by: Andrey Smirnov 
Cc: Jiri Kosina 
Cc: Benjamin Tissoires 
Cc: Henrik Rydberg 
Cc: Pierre-Loup A. Griffais 
Cc: Austin Palmer 
Cc: linux-in...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: sta...@vger.kernel.org # 5.2+
---
 drivers/hid/hid-logitech-hidpp.c | 54 ++--
 1 file changed, 30 insertions(+), 24 deletions(-)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 85911586b3b6..8c4be991f387 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -3498,34 +3498,45 @@ static int hidpp_get_report_length(struct hid_device 
*hdev, int id)
return report->field[0]->report_count + 1;
 }
 
-static bool hidpp_validate_report(struct hid_device *hdev, int id,
- int expected_length, bool optional)
+static bool hidpp_validate_device(struct hid_device *hdev)
 {
-   int report_length;
+   struct hidpp_device *hidpp = hid_get_drvdata(hdev);
+   int id, report_length, supported_reports = 0;
+
+   id = REPORT_ID_HIDPP_SHORT;
+   report_length = hidpp_get_report_length(hdev, id);
+   if (report_length) {
+   if (report_length < HIDPP_REPORT_SHORT_LENGTH)
+   goto bad_device;
 
-   if (id >= HID_MAX_IDS || id < 0) {
-   hid_err(hdev, "invalid HID report id %u\n", id);
-   return false;
+   supported_reports++;
}
 
+   id = REPORT_ID_HIDPP_LONG;
report_length = hidpp_get_report_length(hdev, id);
-   if (!report_length)
-   return optional;
+   if (report_length) {
+   if (report_length < HIDPP_REPORT_LONG_LENGTH)
+   goto bad_device;
 
-   if (report_length < expected_length) {
-   hid_warn(hdev, "not enough values in hidpp report %d\n", id);
-   return false;
+   supported_reports++;
}
 
-   return true;
-}
+   id = REPORT_ID_HIDPP_VERY_LONG;
+   report_length = hidpp_get_report_length(hdev, id);
+   if (report_length) {
+   if (report_length > HIDPP_REPORT_LONG_LENGTH &&
+   report_length < HIDPP_REPORT_VERY_LONG_MAX_LENGTH)
+   goto bad_device;
 
-static bool hidpp_validate_device(struct hid_device *hdev)
-{
-   return hidpp_validate_report(hdev, REPORT_ID_HIDPP_SHORT,
-HIDPP_REPORT_SHORT_LENGTH, false) &&
-  hidpp_validate_report(hdev, REPORT_ID_HIDPP_LONG,
-HIDPP_REPORT_LONG_LENGTH, true);
+   supported_reports++;
+   hidpp->very_long_report_length = report_length;
+   }
+
+   return supported_reports;
+
+bad_device:
+   hid_warn(hdev, "not enough values in hidpp report %d\n", id);
+   return false;
 }
 
 static bool hidpp_application_equals(struct hid_device *hdev,
@@ -3572,11 +3583,6 @@ static int hidpp_probe(struct hid_device *hdev, const 
struct hid_device_id *id)
return hid_hw_start(hdev, HID_CONNECT_DEFAULT);
}
 
-   hidpp->very_long_report_length =
-   hidpp_get_report_length(hdev, REPORT_ID_HIDPP_VERY_LONG);
-   if (hidpp->very_long_report_length > HIDPP_REPORT_VERY_LONG_MAX_LENGTH)
-   hidpp->very_long_report_length = 
HIDPP_REPORT_VERY_LONG_MAX_LENGTH;
-
if (id->group == HID_GROUP_LOGITECH_DJ_DEVICE)
hidpp->quirks |= HIDPP_QUIRK_UNIFYING;
 
-- 
2.21.0

[PATCH v2 0/3] Logitech G920 fixes

[Andrey Smirnov]

Everyone:

This series contains patches to fix a couple of regressions in G920
wheel support by hid-logitech-hidpp driver. Without the patches the
wheel remains stuck in autocentering mode ("resisting" any attempt to
trun) as well as missing support for any FF action.

Thanks,
Andrey Smirnov

Changes since [v1]:

 - "HID: logitech-hidpp: split g920_get_config()" is changed to
   not rely on devres and be a self contained patch

 - Quirk driven behaviour of "HID: logitech-hidpp: add G920 device
   validation quirk" is replaced with generic validation algorithm
   of "HID: logitech-hidpp: rework device validation"

 - Fix for a poteintial race condition is added in
   "HID: logitech-hidpp: do all FF cleanup in hidpp_ff_destroy()"
   as per suggestion by Benjamin Tissoires

 - Collected Tested-by from Sam Bazely for "HID: logitech-hidpp:
   split g920_get_config()" since that patch didn't change
   significantly since [v1]

 - Specified stable kernel versions I think the patches should
   apply to (hopefully I got that right)

[v1] lore.kernel.org/lkml/20191007051240.4410-1-andrew.smir...@gmail.com

Andrey Smirnov (3):
  HID: logitech-hidpp: split g920_get_config()
  HID: logitech-hidpp: rework device validation
  HID: logitech-hidpp: do all FF cleanup in hidpp_ff_destroy()

 drivers/hid/hid-logitech-hidpp.c | 237 +--
 1 file changed, 131 insertions(+), 106 deletions(-)

-- 
2.21.0

[PATCH v2 3/3] HID: logitech-hidpp: do all FF cleanup in hidpp_ff_destroy()

[Andrey Smirnov]

All of the FF-related resources belong to corresponding FF device, so
they should be freed as a part of hidpp_ff_destroy() to avoid
potential race condidions.

Fixes: ff21a635dd1a ("HID: logitech-hidpp: Force feedback support for the 
Logitech G920")
Suggested-by: Benjamin Tissoires 
Signed-off-by: Andrey Smirnov 
Cc: Jiri Kosina 
Cc: Benjamin Tissoires 
Cc: Henrik Rydberg 
Cc: Pierre-Loup A. Griffais 
Cc: Austin Palmer 
Cc: linux-in...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: sta...@vger.kernel.org # 4.9+
---
 drivers/hid/hid-logitech-hidpp.c | 33 +---
 1 file changed, 5 insertions(+), 28 deletions(-)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 8c4be991f387..2524b5104573 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -2078,7 +2078,12 @@ static DEVICE_ATTR(range, S_IRUSR | S_IWUSR | S_IRGRP | 
S_IWGRP | S_IROTH, hidpp
 static void hidpp_ff_destroy(struct ff_device *ff)
 {
struct hidpp_ff_private_data *data = ff->private;
+   struct hid_device *hid = data->hidpp->hid_dev;
 
+   hid_info(hid, "Unloading HID++ force feedback.\n");
+
+   device_remove_file(>dev, _attr_range);
+   destroy_workqueue(data->wq);
kfree(data->effect_ids);
 }
 
@@ -2170,31 +2175,6 @@ static int hidpp_ff_init(struct hidpp_device *hidpp,
return 0;
 }
 
-static int hidpp_ff_deinit(struct hid_device *hid)
-{
-   struct hid_input *hidinput = list_entry(hid->inputs.next, struct 
hid_input, list);
-   struct input_dev *dev = hidinput->input;
-   struct hidpp_ff_private_data *data;
-
-   if (!dev) {
-   hid_err(hid, "Struct input_dev not found!\n");
-   return -EINVAL;
-   }
-
-   hid_info(hid, "Unloading HID++ force feedback.\n");
-   data = dev->ff->private;
-   if (!data) {
-   hid_err(hid, "Private data not found!\n");
-   return -EINVAL;
-   }
-
-   destroy_workqueue(data->wq);
-   device_remove_file(>dev, _attr_range);
-
-   return 0;
-}
-
-
 /* ** 
*/
 /*
*/
 /* Device Support 
*/
@@ -3713,9 +3693,6 @@ static void hidpp_remove(struct hid_device *hdev)
 
sysfs_remove_group(>dev.kobj, _attribute_group);
 
-   if (hidpp->quirks & HIDPP_QUIRK_CLASS_G920)
-   hidpp_ff_deinit(hdev);
-
hid_hw_stop(hdev);
cancel_work_sync(>work);
mutex_destroy(>send_mutex);
-- 
2.21.0

[PATCH v2 1/3] HID: logitech-hidpp: split g920_get_config()

[Andrey Smirnov]

9k_hw 
coretemp snd_hda_codec_hdmi cordic kvm_intel snd_hda_codec_cirrus mac80211 
snd_hda_codec_generic ledtrig_audio kvm snd_hda_intel snd_intel_nhlt irqbypass 
snd_hda_codec btusb btrtl snd_hda_core ath btbcm ssb snd_hwdep btintel snd_seq 
crct10dif_pclmul iTCO_wdt snd_seq_device crc32_pclmul bluetooth mmc_core 
iTCO_vendor_support joydev cfg80211
[   88.320602]  applesmc ghash_clmulni_intel ecdh_generic snd_pcm input_polldev 
intel_cstate ecc intel_uncore thunderbolt snd_timer i2c_i801 libarc4 rfkill 
intel_rapl_perf lpc_ich mei_me pcspkr bcm5974 snd bcma mei soundcore acpi_als 
sbs kfifo_buf sbshc industrialio apple_bl i915 i2c_algo_bit drm_kms_helper drm 
uas crc32c_intel usb_storage video hid_apple
[   88.320630] CR2: 0018
[   88.320633] ---[ end trace 933491c8a4fadeb7 ]---
[   88.320642] RIP: 0010:hidpp_probe+0x61f/0x948 [hid_logitech_hidpp]
[   88.320645] Code: 81 00 00 48 89 ef e8 f0 d6 ff ff 41 89 c6 85 c0 75 b5 0f 
b6 44 24 28 48 8b 5d 00 88 44 24 1e 89 44 24 0c 48 8b 83 18 1c 00 00 <48> 8b 48 
18 48 8b 83 10 19 00 00 48 8b 40 40 48 89 0c 24 0f b7 80
[   88.320647] RSP: 0018:b0a6824aba68 EFLAGS: 00010246
[   88.320650] RAX:  RBX: 93a50756e000 RCX: 00010408
[   88.320652] RDX:  RSI: 93a51f0ad0a0 RDI: 0002d0a0
[   88.320655] RBP: 93a50416da28 R08: 93a50416da70 R09: 93a50416da70
[   88.320657] R10: 00148ae9e60c R11: 000f1525 R12: 93a50756e000
[   88.320659] R13: 93a50756f8d0 R14:  R15: 93a50756fc38
[   88.320662] FS:  7f8d8c1e0940() GS:93a51f08() 
knlGS:
[   88.320664] CS:  0010 DS:  ES:  CR0: 80050033
[   88.320667] CR2: 0018 CR3: 0003996d8003 CR4: 001606e0

To solve this issue:

   1. Split g920_get_config() such that all of the device specific
  communication remains a part of the function and input subsystem
  initialization bits go to hidpp_ff_init()

   2. Move call to hidpp_ff_init() from being a part of
  g920_get_config() to be the last step of .probe(), right after a
  call to hid_hw_start() with connect_mask containing
  HID_CONNECT_HIDINPUT.

Fixes: 91cf9a98ae41 ("HID: logitech-hidpp: make .probe usbhid capable")
Signed-off-by: Andrey Smirnov 
Tested-by: Sam Bazley 
Cc: Jiri Kosina 
Cc: Benjamin Tissoires 
Cc: Henrik Rydberg 
Cc: Pierre-Loup A. Griffais 
Cc: Austin Palmer 
Cc: linux-in...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: sta...@vger.kernel.org # 5.2+
---
 drivers/hid/hid-logitech-hidpp.c | 150 ---
 1 file changed, 96 insertions(+), 54 deletions(-)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 1ac1ecc1e67c..85911586b3b6 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -1669,6 +1669,7 @@ static void hidpp_touchpad_raw_xy_event(struct 
hidpp_device *hidpp_dev,
 
 #define HIDPP_FF_EFFECTID_NONE -1
 #define HIDPP_FF_EFFECTID_AUTOCENTER   -2
+#define HIDPP_AUTOCENTER_PARAMS_LENGTH 18
 
 #define HIDPP_FF_MAX_PARAMS20
 #define HIDPP_FF_RESERVED_SLOTS1
@@ -2009,7 +2010,7 @@ static int hidpp_ff_erase_effect(struct input_dev *dev, 
int effect_id)
 static void hidpp_ff_set_autocenter(struct input_dev *dev, u16 magnitude)
 {
struct hidpp_ff_private_data *data = dev->ff->private;
-   u8 params[18];
+   u8 params[HIDPP_AUTOCENTER_PARAMS_LENGTH];
 
dbg_hid("Setting autocenter to %d.\n", magnitude);
 
@@ -2081,7 +2082,8 @@ static void hidpp_ff_destroy(struct ff_device *ff)
kfree(data->effect_ids);
 }
 
-static int hidpp_ff_init(struct hidpp_device *hidpp, u8 feature_index)
+static int hidpp_ff_init(struct hidpp_device *hidpp,
+struct hidpp_ff_private_data *data)
 {
struct hid_device *hid = hidpp->hid_dev;
struct hid_input *hidinput;
@@ -2089,9 +2091,7 @@ static int hidpp_ff_init(struct hidpp_device *hidpp, u8 
feature_index)
const struct usb_device_descriptor *udesc = 
&(hid_to_usb_dev(hid)->descriptor);
const u16 bcdDevice = le16_to_cpu(udesc->bcdDevice);
struct ff_device *ff;
-   struct hidpp_report response;
-   struct hidpp_ff_private_data *data;
-   int error, j, num_slots;
+   int error, j, num_slots = data->num_effects;
u8 version;
 
if (list_empty(>inputs)) {
@@ -2116,27 +2116,17 @@ static int hidpp_ff_init(struct hidpp_device *hidpp, u8 
feature_index)
for (j = 0; hidpp_ff_effects_v2[j] >= 0; j++)
set_bit(hidpp_ff_effects_v2[j], dev->ffbit);
 
-   /* Read number of slots available in device */
-   error = hidpp_send_fap_command_sync(hidpp, feature_index,
-   HIDPP_FF_GET_INFO, NULL, 0, );
-   if (error) {
-   if (error < 0)
-   return error;

[PATCH 4/4] arm64: dts: zii-ultra: Add node for switch watchdog

[Andrey Smirnov]

Add I2C node for switch watchdog present on both Zest and RMB3 boards.

Signed-off-by: Andrey Smirnov 
Cc: Fabio Estevam 
Cc: Chris Healy 
Cc: Lucas Stach 
Cc: Shawn Guo 
Cc: linux-arm-ker...@lists.infradead.org,
Cc: linux-kernel@vger.kernel.org
---
 arch/arm64/boot/dts/freescale/imx8mq-zii-ultra.dtsi | 5 +
 1 file changed, 5 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mq-zii-ultra.dtsi 
b/arch/arm64/boot/dts/freescale/imx8mq-zii-ultra.dtsi
index 8395c5a73ba6..e058ad908b2e 100644
--- a/arch/arm64/boot/dts/freescale/imx8mq-zii-ultra.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mq-zii-ultra.dtsi
@@ -400,6 +400,11 @@
reg = <0x2c>;
reset-gpios = < 25 GPIO_ACTIVE_LOW>;
};
+
+   watchdog@38 {
+   compatible = "zii,rave-wdt";
+   reg = <0x38>;
+   };
 };
 
  {
-- 
2.21.0

[PATCH 3/4] arm64: dts: zii-ultra: Add node for accelerometer

[Andrey Smirnov]

Add I2C node for accelerometer present on both Zest and RMB3 boards.

Signed-off-by: Andrey Smirnov 
Cc: Fabio Estevam 
Cc: Chris Healy 
Cc: Lucas Stach 
Cc: Shawn Guo 
Cc: linux-arm-ker...@lists.infradead.org,
Cc: linux-kernel@vger.kernel.org
---
 .../boot/dts/freescale/imx8mq-zii-ultra.dtsi   | 18 ++
 1 file changed, 18 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mq-zii-ultra.dtsi 
b/arch/arm64/boot/dts/freescale/imx8mq-zii-ultra.dtsi
index 21eb52341ba8..8395c5a73ba6 100644
--- a/arch/arm64/boot/dts/freescale/imx8mq-zii-ultra.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mq-zii-ultra.dtsi
@@ -262,6 +262,18 @@
pinctrl-0 = <_i2c1>;
status = "okay";
 
+   accel@1c {
+   compatible = "fsl,mma8451";
+   pinctrl-names = "default";
+   pinctrl-0 = <_accel>;
+   reg = <0x1c>;
+   interrupt-parent = <>;
+   interrupts = <20 IRQ_TYPE_LEVEL_LOW>;
+   interrupt-names = "INT2";
+   vdd-supply = <_gen_3p3>;
+   vddio-supply = <_gen_3p3>;
+   };
+
ucs1002: charger@32 {
compatible = "microchip,ucs1002";
pinctrl-names = "default";
@@ -522,6 +534,12 @@
 };
 
  {
+   pinctrl_accel: accelgrp {
+   fsl,pins = <
+   MX8MQ_IOMUXC_SAI5_RXC_GPIO3_IO200x41
+   >;
+   };
+
pinctrl_fec1: fec1grp {
fsl,pins = <
MX8MQ_IOMUXC_ENET_MDC_ENET1_MDC 0x3
-- 
2.21.0

[PATCH 2/4] arm64: dts: zii-ultra: Fix regulator-3p3-main's name

[Andrey Smirnov]

It's 3V3_MAIN, not 3V3V_MAIN on schematic. Fix it.

Signed-off-by: Andrey Smirnov 
Cc: Fabio Estevam 
Cc: Chris Healy 
Cc: Lucas Stach 
Cc: Shawn Guo 
Cc: linux-arm-ker...@lists.infradead.org,
Cc: linux-kernel@vger.kernel.org
---
 arch/arm64/boot/dts/freescale/imx8mq-zii-ultra.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/freescale/imx8mq-zii-ultra.dtsi 
b/arch/arm64/boot/dts/freescale/imx8mq-zii-ultra.dtsi
index 5d7a8f09f1ab..21eb52341ba8 100644
--- a/arch/arm64/boot/dts/freescale/imx8mq-zii-ultra.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mq-zii-ultra.dtsi
@@ -62,7 +62,7 @@
reg_3p3_main: regulator-3p3-main {
compatible = "regulator-fixed";
vin-supply = <_12p0_main>;
-   regulator-name = "3V3V_MAIN";
+   regulator-name = "3V3_MAIN";
regulator-min-microvolt = <330>;
regulator-max-microvolt = <330>;
regulator-always-on;
-- 
2.21.0

[PATCH 1/4] arm64: dts: zii-ultra: Fix regulator-vsd-3v3's vin-supply

[Andrey Smirnov]

Regulator-vsd-3v3 is supplied via GEN_3V3 rail which is an output of
an "always on" load switch supplied by 3V3_MAIN. GEN_3V3 is also used
as vin-supply by a number of peripherals, so adding it also allows us
to follow the schematic more closely.

Signed-off-by: Andrey Smirnov 
Cc: Fabio Estevam 
Cc: Chris Healy 
Cc: Lucas Stach 
Cc: Shawn Guo 
Cc: linux-arm-ker...@lists.infradead.org,
Cc: linux-kernel@vger.kernel.org
---
 arch/arm64/boot/dts/freescale/imx8mq-zii-ultra.dtsi | 11 ++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/freescale/imx8mq-zii-ultra.dtsi 
b/arch/arm64/boot/dts/freescale/imx8mq-zii-ultra.dtsi
index 087b5b6ebe89..5d7a8f09f1ab 100644
--- a/arch/arm64/boot/dts/freescale/imx8mq-zii-ultra.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mq-zii-ultra.dtsi
@@ -68,11 +68,20 @@
regulator-always-on;
};
 
+   reg_gen_3p3: regulator-gen-3p3 {
+   compatible = "regulator-fixed";
+   vin-supply = <_3p3_main>;
+   regulator-name = "GEN_3V3";
+   regulator-min-microvolt = <330>;
+   regulator-max-microvolt = <330>;
+   regulator-always-on;
+   };
+
reg_usdhc2_vmmc: regulator-vsd-3v3 {
pinctrl-names = "default";
pinctrl-0 = <_reg_usdhc2>;
compatible = "regulator-fixed";
-   vin-supply = <_3p3_main>;
+   vin-supply = <_gen_3p3>;
regulator-name = "3V3_SD";
regulator-min-microvolt = <330>;
regulator-max-microvolt = <330>;
-- 
2.21.0

Re: [PATCH 1/3] HID: logitech-hidpp: use devres to manage FF private data

[Andrey Smirnov]

On Sun, Oct 13, 2019 at 8:54 PM Sasha Levin  wrote:
>
> Hi,
>
> [This is an automated email]
>
> This commit has been processed because it contains a -stable tag.
> The stable tag indicates that it's relevant for the following trees: all
>
> The bot has tested the following trees: v5.3.5, v5.2.20, v4.19.78, v4.14.148, 
> v4.9.196, v4.4.196.
>
> v5.3.5: Build OK!
> v5.2.20: Build OK!
> v4.19.78: Failed to apply! Possible dependencies:
> 43cd97af70c65 ("HID: logitech: Stop setting drvdata to NULL on probe 
> failure and remove")
>
> v4.14.148: Failed to apply! Possible dependencies:
> 43cd97af70c65 ("HID: logitech: Stop setting drvdata to NULL on probe 
> failure and remove")
>
> v4.9.196: Failed to apply! Possible dependencies:
> 43cd97af70c65 ("HID: logitech: Stop setting drvdata to NULL on probe 
> failure and remove")
>
> v4.4.196: Failed to apply! Possible dependencies:
> 43cd97af70c65 ("HID: logitech: Stop setting drvdata to NULL on probe 
> failure and remove")
> 6c44b15e1c907 ("HID: logitech: check the return value of 
> create_singlethread_workqueue")
> 7bfd2927adcac ("HID: hid-logitech-hidpp: Add basic support for Logitech 
> G920")
> 7f4b49fef6ffb ("HID: hid-logitech-hidpp: Add range sysfs for Logitech 
> G920")
> af2e628d6be7a ("HID: logitech-hidpp: limit visibility of init/deinit 
> functions")
> ff21a635dd1a9 ("HID: logitech-hidpp: Force feedback support for the 
> Logitech G920")
>
>
> NOTE: The patch will not be queued to stable trees until it is upstream.
>
> How should we proceed with this patch?

Please ignore this series, since it will be superseded by upcoming v2

Thanks,
Andrey Smirnov

Re: [PATCH 3/3] HID: logitech-hidpp: add G920 device validation quirk

[Andrey Smirnov]

On Fri, Oct 11, 2019 at 3:33 PM Benjamin Tissoires
 wrote:
>
> On Fri, Oct 11, 2019 at 9:39 PM Andrey Smirnov  
> wrote:
> >
> > On Fri, Oct 11, 2019 at 7:56 AM Benjamin Tissoires
> >  wrote:
> > >
> > > On Mon, Oct 7, 2019 at 7:13 AM Andrey Smirnov  
> > > wrote:
> > > >
> > > > G920 device only advertises REPORT_ID_HIDPP_LONG and
> > > > REPORT_ID_HIDPP_VERY_LONG in its HID report descriptor, so querying
> > > > for REPORT_ID_HIDPP_SHORT with optional=false will always fail and
> > > > prevent G920 to be recognized as a valid HID++ device.
> > > >
> > > > Modify hidpp_validate_device() to check only REPORT_ID_HIDPP_LONG with
> > > > optional=false on G920 to fix this.
> > > >
> > > > Fixes: fe3ee1ec007b ("HID: logitech-hidpp: allow non HID++ devices to 
> > > > be handled by this module")
> > > > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=204191
> > > > Reported-by: Sam Bazely 
> > > > Signed-off-by: Andrey Smirnov 
> > > > Cc: Jiri Kosina 
> > > > Cc: Benjamin Tissoires 
> > > > Cc: Henrik Rydberg 
> > > > Cc: Sam Bazely 
> > > > Cc: Pierre-Loup A. Griffais 
> > > > Cc: Austin Palmer 
> > > > Cc: linux-in...@vger.kernel.org
> > > > Cc: linux-kernel@vger.kernel.org
> > > > Cc: sta...@vger.kernel.org
> > > > ---
> > > >  drivers/hid/hid-logitech-hidpp.c | 6 ++
> > > >  1 file changed, 6 insertions(+)
> > > >
> > > > diff --git a/drivers/hid/hid-logitech-hidpp.c 
> > > > b/drivers/hid/hid-logitech-hidpp.c
> > > > index cadf36d6c6f3..f415bf398e17 100644
> > > > --- a/drivers/hid/hid-logitech-hidpp.c
> > > > +++ b/drivers/hid/hid-logitech-hidpp.c
> > > > @@ -3511,6 +3511,12 @@ static bool hidpp_validate_report(struct 
> > > > hid_device *hdev, int id,
> > > >
> > > >  static bool hidpp_validate_device(struct hid_device *hdev)
> > > >  {
> > > > +   struct hidpp_device *hidpp = hid_get_drvdata(hdev);
> > > > +
> > > > +   if (hidpp->quirks & HIDPP_QUIRK_CLASS_G920)
> > > > +   return hidpp_validate_report(hdev, REPORT_ID_HIDPP_LONG,
> > > > +HIDPP_REPORT_SHORT_LENGTH, 
> > > > false);
> > > > +
> > >
> > > with https://patchwork.kernel.org/patch/11184749/ we also have a need
> > > for such a trick for BLE mice.
> > >
> > > I wonder if we should not have a more common way of validating the devices
> > >
> >
> > What about just checking for:
> >
> > hidpp_validate_report(REPORT_ID_HIDPP_SHORT,
> > HIDPP_REPORT_SHORT_LENGTH, true) ||
> > hidpp_validate_report(hdev, REPORT_ID_HIDPP_LONG,
> > HIDPP_REPORT_LONG_LENGTH, true);
> >
> > and probably dropping the "optional" argument for
> > hidpp_validate_report()? Original code allows there to be devices
> > supporting shorts reports only, but it seems that devices that support
> > only long reports are legitimate too, so maybe the only "invalid"
> > combination is if both are invalid length or missing?
>
> Well, the problem is we also want to detect 2 things:
> - devices that do not have any of the HID++ collections, and handle
> them as generic ones (the second mouse/keyboard collection in the
> gaming mice should still be exported by the driver, or this will kill
> the macros / rebinding capabilities
> - malicious devices that pretends to have a HID++ collection but want
> to trigger a buffer overflow by having a shorter than expected report
> length
>
> Point 2 above should still be fine, but point 1 is why we have the
> enforcement of the HID++ short report in the first place.
>

It sounds like the result of hidpp_validate_report() can't really be
contained in a bool. If we modify it to return -EINVAL for bogus
report length, -ENOTSUPP if report ID is not supported and 0 if
everything is valid we should be able to capture all valid permutation
by checking for with

int id_short = hidpp_validate_report(ID_SHORT);
int id_long  = hidpp_validate_report(ID_LONG);

return (!id_short && !id_long) || (id_short == -ENOTSUPP && !id_long)
|| (id_long == -ENOTSUPP && !id_short)

no?

Thanks,
Andrey Smirnov

Re: [PATCH 1/3] HID: logitech-hidpp: use devres to manage FF private data

[Andrey Smirnov]

On Fri, Oct 11, 2019 at 1:33 PM Dmitry Torokhov
 wrote:
>
> On Fri, Oct 11, 2019 at 09:25:52PM +0200, Benjamin Tissoires wrote:
> > On Fri, Oct 11, 2019 at 8:26 PM Dmitry Torokhov
> >  wrote:
> > >
> > > On Fri, Oct 11, 2019 at 04:52:04PM +0200, Benjamin Tissoires wrote:
> > > > Hi Andrey,
> > > >
> > > > On Mon, Oct 7, 2019 at 7:13 AM Andrey Smirnov 
> > > >  wrote:
> > > > >
> > > > > To simplify resource management in commit that follows as well as to
> > > > > save a couple of extra kfree()s and simplify hidpp_ff_deinit() switch
> > > > > driver code to use devres to manage the life-cycle of FF private data.
> > > > >
> > > > > Signed-off-by: Andrey Smirnov 
> > > > > Cc: Jiri Kosina 
> > > > > Cc: Benjamin Tissoires 
> > > > > Cc: Henrik Rydberg 
> > > > > Cc: Sam Bazely 
> > > > > Cc: Pierre-Loup A. Griffais 
> > > > > Cc: Austin Palmer 
> > > > > Cc: linux-in...@vger.kernel.org
> > > > > Cc: linux-kernel@vger.kernel.org
> > > > > Cc: sta...@vger.kernel.org
> > > >
> > > > This patch doesn't seem to fix any error, is there a reason to send it
> > > > to stable? (besides as a dependency of the rest of the series).
> > > >
> > > > > ---
> > > > >  drivers/hid/hid-logitech-hidpp.c | 53 
> > > > > +---
> > > > >  1 file changed, 29 insertions(+), 24 deletions(-)
> > > > >
> > > > > diff --git a/drivers/hid/hid-logitech-hidpp.c 
> > > > > b/drivers/hid/hid-logitech-hidpp.c
> > > > > index 0179f7ed77e5..58eb928224e5 100644
> > > > > --- a/drivers/hid/hid-logitech-hidpp.c
> > > > > +++ b/drivers/hid/hid-logitech-hidpp.c
> > > > > @@ -2079,6 +2079,11 @@ static void hidpp_ff_destroy(struct ff_device 
> > > > > *ff)
> > > > > struct hidpp_ff_private_data *data = ff->private;
> > > > >
> > > > > kfree(data->effect_ids);
> > > >
> > > > Is there any reasons we can not also devm alloc data->effect_ids?
> > > >
> > > > > +   /*
> > > > > +* Set private to NULL to prevent input_ff_destroy() from
> > > > > +* freeing our devres allocated memory
> > > >
> > > > Ouch. There is something wrong here: input_ff_destroy() calls
> > > > kfree(ff->private), when the data has not been allocated by
> > > > input_ff_create(). This seems to lack a little bit of symmetry.
> > >
> > > Yeah, ff and ff-memless essentially take over the private data assigned
> > > to them. They were done before devm and the lifetime of the "private"
> > > data pieces was tied to the lifetime of the input device to simplify
> > > error handling and teardown.
> >
> > Yeah, that stealing of the pointer is not good :)
> > But OTOH, it helps
> >
> > >
> > > Maybe we should clean it up a bit... I'm open to suggestions.
> >
> > The problem I had when doing the review was that there is no easy way
> > to have a "devm_input_ff_create_()", because the way it's built is
> > already "devres-compatible": the destroy gets called by input core.
>
> I do not think we want devm_input_ff_create() explicitly, I think the
> fact that you can "build up" an input device by allocating it, then
> adding slots, poller, ff support, etc, and input core cleans it up is
> all good. It is just the ownership if the driver-private data block is
> not very obvious and is not compatible with allocating via devm.
>

What do you think of creating input_ff_create_with_private() which
would take a pointer to private memory and set a new
"borrowed_private" flag in struct ff_device which input_ff_destroy()
can check to determine if it needs to free ->private or not?

Thanks,
Andrey Smirnov

Re: [PATCH 1/3] HID: logitech-hidpp: use devres to manage FF private data

[Andrey Smirnov]

On Fri, Oct 11, 2019 at 12:26 PM Benjamin Tissoires
 wrote:
>
> On Fri, Oct 11, 2019 at 8:26 PM Dmitry Torokhov
>  wrote:
> >
> > On Fri, Oct 11, 2019 at 04:52:04PM +0200, Benjamin Tissoires wrote:
> > > Hi Andrey,
> > >
> > > On Mon, Oct 7, 2019 at 7:13 AM Andrey Smirnov  
> > > wrote:
> > > >
> > > > To simplify resource management in commit that follows as well as to
> > > > save a couple of extra kfree()s and simplify hidpp_ff_deinit() switch
> > > > driver code to use devres to manage the life-cycle of FF private data.
> > > >
> > > > Signed-off-by: Andrey Smirnov 
> > > > Cc: Jiri Kosina 
> > > > Cc: Benjamin Tissoires 
> > > > Cc: Henrik Rydberg 
> > > > Cc: Sam Bazely 
> > > > Cc: Pierre-Loup A. Griffais 
> > > > Cc: Austin Palmer 
> > > > Cc: linux-in...@vger.kernel.org
> > > > Cc: linux-kernel@vger.kernel.org
> > > > Cc: sta...@vger.kernel.org
> > >
> > > This patch doesn't seem to fix any error, is there a reason to send it
> > > to stable? (besides as a dependency of the rest of the series).
> > >
> > > > ---
> > > >  drivers/hid/hid-logitech-hidpp.c | 53 +---
> > > >  1 file changed, 29 insertions(+), 24 deletions(-)
> > > >
> > > > diff --git a/drivers/hid/hid-logitech-hidpp.c 
> > > > b/drivers/hid/hid-logitech-hidpp.c
> > > > index 0179f7ed77e5..58eb928224e5 100644
> > > > --- a/drivers/hid/hid-logitech-hidpp.c
> > > > +++ b/drivers/hid/hid-logitech-hidpp.c
> > > > @@ -2079,6 +2079,11 @@ static void hidpp_ff_destroy(struct ff_device 
> > > > *ff)
> > > > struct hidpp_ff_private_data *data = ff->private;
> > > >
> > > > kfree(data->effect_ids);
> > >
> > > Is there any reasons we can not also devm alloc data->effect_ids?
> > >
> > > > +   /*
> > > > +* Set private to NULL to prevent input_ff_destroy() from
> > > > +* freeing our devres allocated memory
> > >
> > > Ouch. There is something wrong here: input_ff_destroy() calls
> > > kfree(ff->private), when the data has not been allocated by
> > > input_ff_create(). This seems to lack a little bit of symmetry.
> >
> > Yeah, ff and ff-memless essentially take over the private data assigned
> > to them. They were done before devm and the lifetime of the "private"
> > data pieces was tied to the lifetime of the input device to simplify
> > error handling and teardown.
>
> Yeah, that stealing of the pointer is not good :)
> But OTOH, it helps
>
> >
> > Maybe we should clean it up a bit... I'm open to suggestions.
>
> The problem I had when doing the review was that there is no easy way
> to have a "devm_input_ff_create_()", because the way it's built is
> already "devres-compatible": the destroy gets called by input core.
>
> So I don't have a good answer to simplify in a transparent manner
> without breaking the API.
>
> >
> > In this case maybe best way is to get rid of hidpp_ff_destroy() and not
> > set ff->private and rely on devm to free the buffers. One can get to
> > device private data from ff methods via input_get_drvdata() since they
> > all (except destroy) are passed input device pointer.
>
> Sounds like a good idea. However, it seems there might be a race when
> removing the workqueue:
> the workqueue gets deleted in hidpp_remove, when the input node will
> be freed by devres, so after the call of hidpp_remove.
>
> So we should probably keep hidpp_ff_destroy() to clean up the ff bits,
> and instead move the content of hidpp_ff_deinit() into
> hidpp_ff_destroy() so we ensure proper ordering.
>
> Andrey, note that ensuring the workqueue gets freed after the call of
> input_destroy_device is something that should definitively go into
> stable as this is a potential race problem.

Sure, I'll add this to the series as a separate patch.

Thanks,
Andrey Smirnov

Re: [PATCH 3/3] HID: logitech-hidpp: add G920 device validation quirk

[Andrey Smirnov]

On Fri, Oct 11, 2019 at 7:56 AM Benjamin Tissoires
 wrote:
>
> On Mon, Oct 7, 2019 at 7:13 AM Andrey Smirnov  
> wrote:
> >
> > G920 device only advertises REPORT_ID_HIDPP_LONG and
> > REPORT_ID_HIDPP_VERY_LONG in its HID report descriptor, so querying
> > for REPORT_ID_HIDPP_SHORT with optional=false will always fail and
> > prevent G920 to be recognized as a valid HID++ device.
> >
> > Modify hidpp_validate_device() to check only REPORT_ID_HIDPP_LONG with
> > optional=false on G920 to fix this.
> >
> > Fixes: fe3ee1ec007b ("HID: logitech-hidpp: allow non HID++ devices to be 
> > handled by this module")
> > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=204191
> > Reported-by: Sam Bazely 
> > Signed-off-by: Andrey Smirnov 
> > Cc: Jiri Kosina 
> > Cc: Benjamin Tissoires 
> > Cc: Henrik Rydberg 
> > Cc: Sam Bazely 
> > Cc: Pierre-Loup A. Griffais 
> > Cc: Austin Palmer 
> > Cc: linux-in...@vger.kernel.org
> > Cc: linux-kernel@vger.kernel.org
> > Cc: sta...@vger.kernel.org
> > ---
> >  drivers/hid/hid-logitech-hidpp.c | 6 ++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/drivers/hid/hid-logitech-hidpp.c 
> > b/drivers/hid/hid-logitech-hidpp.c
> > index cadf36d6c6f3..f415bf398e17 100644
> > --- a/drivers/hid/hid-logitech-hidpp.c
> > +++ b/drivers/hid/hid-logitech-hidpp.c
> > @@ -3511,6 +3511,12 @@ static bool hidpp_validate_report(struct hid_device 
> > *hdev, int id,
> >
> >  static bool hidpp_validate_device(struct hid_device *hdev)
> >  {
> > +   struct hidpp_device *hidpp = hid_get_drvdata(hdev);
> > +
> > +   if (hidpp->quirks & HIDPP_QUIRK_CLASS_G920)
> > +   return hidpp_validate_report(hdev, REPORT_ID_HIDPP_LONG,
> > +HIDPP_REPORT_SHORT_LENGTH, 
> > false);
> > +
>
> with https://patchwork.kernel.org/patch/11184749/ we also have a need
> for such a trick for BLE mice.
>
> I wonder if we should not have a more common way of validating the devices
>

What about just checking for:

hidpp_validate_report(REPORT_ID_HIDPP_SHORT,
HIDPP_REPORT_SHORT_LENGTH, true) ||
hidpp_validate_report(hdev, REPORT_ID_HIDPP_LONG,
HIDPP_REPORT_LONG_LENGTH, true);

and probably dropping the "optional" argument for
hidpp_validate_report()? Original code allows there to be devices
supporting shorts reports only, but it seems that devices that support
only long reports are legitimate too, so maybe the only "invalid"
combination is if both are invalid length or missing?

Thanks,
Andrey Smirnov

Re: [PATCH 1/3] HID: logitech-hidpp: use devres to manage FF private data

[Andrey Smirnov]

On Fri, Oct 11, 2019 at 7:52 AM Benjamin Tissoires
 wrote:
>
> Hi Andrey,
>
> On Mon, Oct 7, 2019 at 7:13 AM Andrey Smirnov  
> wrote:
> >
> > To simplify resource management in commit that follows as well as to
> > save a couple of extra kfree()s and simplify hidpp_ff_deinit() switch
> > driver code to use devres to manage the life-cycle of FF private data.
> >
> > Signed-off-by: Andrey Smirnov 
> > Cc: Jiri Kosina 
> > Cc: Benjamin Tissoires 
> > Cc: Henrik Rydberg 
> > Cc: Sam Bazely 
> > Cc: Pierre-Loup A. Griffais 
> > Cc: Austin Palmer 
> > Cc: linux-in...@vger.kernel.org
> > Cc: linux-kernel@vger.kernel.org
> > Cc: sta...@vger.kernel.org
>
> This patch doesn't seem to fix any error, is there a reason to send it
> to stable? (besides as a dependency of the rest of the series).
>

Dependency is the only reason for this patch, but it is a pretty big
reason. Prior to patches 1/3 and 2/3 FF private data was both
allocated and passed off to FF layer via ff->private = data in a span
of a few lines of code within hidpp_ff_init()/g920_get_config().
After, said pair is effectively split into two different functions,
both needing access to FF private data, but called quite far apart in
hidpp_probe(). Alternatives to patch 1/3 would be to either make sure
that every error path in hidpp_prob() after the call to
g920_allocate() is aware of allocated FF data, which seems like a
nightmare, or, to create a temporary FF data, fill it in
g920_get_config() and memdup() it in hidpp_ff_init(). Is that (the
latter) the path that you prefer to take?

> > ---
> >  drivers/hid/hid-logitech-hidpp.c | 53 +---
> >  1 file changed, 29 insertions(+), 24 deletions(-)
> >
> > diff --git a/drivers/hid/hid-logitech-hidpp.c 
> > b/drivers/hid/hid-logitech-hidpp.c
> > index 0179f7ed77e5..58eb928224e5 100644
> > --- a/drivers/hid/hid-logitech-hidpp.c
> > +++ b/drivers/hid/hid-logitech-hidpp.c
> > @@ -2079,6 +2079,11 @@ static void hidpp_ff_destroy(struct ff_device *ff)
> > struct hidpp_ff_private_data *data = ff->private;
> >
> > kfree(data->effect_ids);
>
> Is there any reasons we can not also devm alloc data->effect_ids?

No, but I was trying to limit the scope of this patch.

>
> > +   /*
> > +* Set private to NULL to prevent input_ff_destroy() from
> > +* freeing our devres allocated memory
>
> Ouch. There is something wrong here: input_ff_destroy() calls
> kfree(ff->private), when the data has not been allocated by
> input_ff_create(). This seems to lack a little bit of symmetry.
>

I agree, I think it's a wart in FF API design.

> > +*/
> > +   ff->private = NULL;
> >  }
> >
> >  static int hidpp_ff_init(struct hidpp_device *hidpp, u8 feature_index)
> > @@ -2090,7 +2095,7 @@ static int hidpp_ff_init(struct hidpp_device *hidpp, 
> > u8 feature_index)
> > const u16 bcdDevice = le16_to_cpu(udesc->bcdDevice);
> > struct ff_device *ff;
> > struct hidpp_report response;
> > -   struct hidpp_ff_private_data *data;
> > +   struct hidpp_ff_private_data *data = hidpp->private_data;
> > int error, j, num_slots;
> > u8 version;
> >
> > @@ -2129,18 +2134,13 @@ static int hidpp_ff_init(struct hidpp_device 
> > *hidpp, u8 feature_index)
> > return error;
> > }
> >
> > -   data = kzalloc(sizeof(*data), GFP_KERNEL);
> > -   if (!data)
> > -   return -ENOMEM;
> > data->effect_ids = kcalloc(num_slots, sizeof(int), GFP_KERNEL);
> > -   if (!data->effect_ids) {
> > -   kfree(data);
> > +   if (!data->effect_ids)
> > return -ENOMEM;
> > -   }
> > +
> > data->wq = create_singlethread_workqueue("hidpp-ff-sendqueue");
> > if (!data->wq) {
> > kfree(data->effect_ids);
> > -   kfree(data);
> > return -ENOMEM;
> > }
> >
> > @@ -2199,28 +2199,15 @@ static int hidpp_ff_init(struct hidpp_device 
> > *hidpp, u8 feature_index)
> > return 0;
> >  }
> >
> > -static int hidpp_ff_deinit(struct hid_device *hid)
> > +static void hidpp_ff_deinit(struct hid_device *hid)
> >  {
> > -   struct hid_input *hidinput = list_entry(hid->inputs.next, struct 
> > hid_input, list);
> > -   struct input_dev *dev = hidinput->input;
> > -   struct hidpp_ff_private_data *data;
> > -
> > -   

Re: [PATCH 0/3] Logitech G920 fixes

[Andrey Smirnov]

On Fri, Oct 11, 2019 at 7:53 AM Benjamin Tissoires
 wrote:
>
> Hi,
>
> Finally, someone who takes care of the G920!
> Note that when I sent my first initial version that Hans reused, I
> surely broke it (looking at your patch 3/3), but no one cared to test
> it :(
>
> On Mon, Oct 7, 2019 at 7:13 AM Andrey Smirnov  
> wrote:
> >
> > Everyone:
> >
> > This series contains patches to fix a couple of regressions in G920
> > wheel support by hid-logitech-hidpp driver. Without the patches the
> > wheel remains stuck in autocentering mode ("resisting" any attempt to
> > trun) as well as missing support for any FF action.
>
> So, you are talking about regressions, and for that I would like to be
> able to push the patches to stable.
>
> However, I would need more information:
> - patch 3/3 seems simple enough to go in stable, and is clearly a
> regression from the recent series. Can you put it in first and add
> sta...@vger.kernel.org in a CC field (and possibly with a Fixes tag as
> well)?

It patch 3/3 on purpose because applying it by itself, without fix in
2/3 in place would lead to a segfault and a non working wheel. Maybe
that FF for-next fix you pointed out can prevent that from happening,
but as is the series is pretty atomic and can't be divided.

Patch 3/3 already has stable in CC and Fixes tag.

> - I am not sure which patch fixes the wheel remains stuck in
> autocentering mode. Is it patch 2/3?

There's no specific patch that does that. There were two G920
regressions in the driver and both need to be fixed for wheel to be
configured properly. The specific code that releases the wheel is in
g920_ff_set_autocenter().

> - was the "wheel remains stuck in autocentering mode" bug present from
> on of the recent patch or was it always there since we introduced
> support in hid-logitech-hidpp, but the game would need to unlock the
> wheel first?

The wheel worked as expected prior to

fe3ee1ec007b ("HID: logitech-hidpp: allow non HID++ devices to be
handled by this module")
91cf9a98ae41 ("HID: logitech-hidpp: make .probe usbhid capable")

It's not the game that needs to unlock the wheel, but the driver itself.

Thanks,
Andrey Smirnov

[PATCH 3/3] HID: logitech-hidpp: add G920 device validation quirk

[Andrey Smirnov]

G920 device only advertises REPORT_ID_HIDPP_LONG and
REPORT_ID_HIDPP_VERY_LONG in its HID report descriptor, so querying
for REPORT_ID_HIDPP_SHORT with optional=false will always fail and
prevent G920 to be recognized as a valid HID++ device.

Modify hidpp_validate_device() to check only REPORT_ID_HIDPP_LONG with
optional=false on G920 to fix this.

Fixes: fe3ee1ec007b ("HID: logitech-hidpp: allow non HID++ devices to be 
handled by this module")
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=204191
Reported-by: Sam Bazely 
Signed-off-by: Andrey Smirnov 
Cc: Jiri Kosina 
Cc: Benjamin Tissoires 
Cc: Henrik Rydberg 
Cc: Sam Bazely 
Cc: Pierre-Loup A. Griffais 
Cc: Austin Palmer 
Cc: linux-in...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: sta...@vger.kernel.org
---
 drivers/hid/hid-logitech-hidpp.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index cadf36d6c6f3..f415bf398e17 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -3511,6 +3511,12 @@ static bool hidpp_validate_report(struct hid_device 
*hdev, int id,
 
 static bool hidpp_validate_device(struct hid_device *hdev)
 {
+   struct hidpp_device *hidpp = hid_get_drvdata(hdev);
+
+   if (hidpp->quirks & HIDPP_QUIRK_CLASS_G920)
+   return hidpp_validate_report(hdev, REPORT_ID_HIDPP_LONG,
+HIDPP_REPORT_SHORT_LENGTH, false);
+
return hidpp_validate_report(hdev, REPORT_ID_HIDPP_SHORT,
 HIDPP_REPORT_SHORT_LENGTH, false) &&
   hidpp_validate_report(hdev, REPORT_ID_HIDPP_LONG,
-- 
2.21.0

[PATCH 1/3] HID: logitech-hidpp: use devres to manage FF private data

[Andrey Smirnov]

To simplify resource management in commit that follows as well as to
save a couple of extra kfree()s and simplify hidpp_ff_deinit() switch
driver code to use devres to manage the life-cycle of FF private data.

Signed-off-by: Andrey Smirnov 
Cc: Jiri Kosina 
Cc: Benjamin Tissoires 
Cc: Henrik Rydberg 
Cc: Sam Bazely 
Cc: Pierre-Loup A. Griffais 
Cc: Austin Palmer 
Cc: linux-in...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: sta...@vger.kernel.org
---
 drivers/hid/hid-logitech-hidpp.c | 53 +---
 1 file changed, 29 insertions(+), 24 deletions(-)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 0179f7ed77e5..58eb928224e5 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -2079,6 +2079,11 @@ static void hidpp_ff_destroy(struct ff_device *ff)
struct hidpp_ff_private_data *data = ff->private;
 
kfree(data->effect_ids);
+   /*
+* Set private to NULL to prevent input_ff_destroy() from
+* freeing our devres allocated memory
+*/
+   ff->private = NULL;
 }
 
 static int hidpp_ff_init(struct hidpp_device *hidpp, u8 feature_index)
@@ -2090,7 +2095,7 @@ static int hidpp_ff_init(struct hidpp_device *hidpp, u8 
feature_index)
const u16 bcdDevice = le16_to_cpu(udesc->bcdDevice);
struct ff_device *ff;
struct hidpp_report response;
-   struct hidpp_ff_private_data *data;
+   struct hidpp_ff_private_data *data = hidpp->private_data;
int error, j, num_slots;
u8 version;
 
@@ -2129,18 +2134,13 @@ static int hidpp_ff_init(struct hidpp_device *hidpp, u8 
feature_index)
return error;
}
 
-   data = kzalloc(sizeof(*data), GFP_KERNEL);
-   if (!data)
-   return -ENOMEM;
data->effect_ids = kcalloc(num_slots, sizeof(int), GFP_KERNEL);
-   if (!data->effect_ids) {
-   kfree(data);
+   if (!data->effect_ids)
return -ENOMEM;
-   }
+
data->wq = create_singlethread_workqueue("hidpp-ff-sendqueue");
if (!data->wq) {
kfree(data->effect_ids);
-   kfree(data);
return -ENOMEM;
}
 
@@ -2199,28 +2199,15 @@ static int hidpp_ff_init(struct hidpp_device *hidpp, u8 
feature_index)
return 0;
 }
 
-static int hidpp_ff_deinit(struct hid_device *hid)
+static void hidpp_ff_deinit(struct hid_device *hid)
 {
-   struct hid_input *hidinput = list_entry(hid->inputs.next, struct 
hid_input, list);
-   struct input_dev *dev = hidinput->input;
-   struct hidpp_ff_private_data *data;
-
-   if (!dev) {
-   hid_err(hid, "Struct input_dev not found!\n");
-   return -EINVAL;
-   }
+   struct hidpp_device *hidpp = hid_get_drvdata(hid);
+   struct hidpp_ff_private_data *data = hidpp->private_data;
 
hid_info(hid, "Unloading HID++ force feedback.\n");
-   data = dev->ff->private;
-   if (!data) {
-   hid_err(hid, "Private data not found!\n");
-   return -EINVAL;
-   }
 
destroy_workqueue(data->wq);
device_remove_file(>dev, _attr_range);
-
-   return 0;
 }
 
 
@@ -2725,6 +2712,20 @@ static int k400_connect(struct hid_device *hdev, bool 
connected)
 
 #define HIDPP_PAGE_G920_FORCE_FEEDBACK 0x8123
 
+static int g920_allocate(struct hid_device *hdev)
+{
+   struct hidpp_device *hidpp = hid_get_drvdata(hdev);
+   struct hidpp_ff_private_data *data;
+
+   data = devm_kzalloc(>dev, sizeof(*data), GFP_KERNEL);
+   if (!data)
+   return -ENOMEM;
+
+   hidpp->private_data = data;
+
+   return 0;
+}
+
 static int g920_get_config(struct hidpp_device *hidpp)
 {
u8 feature_type;
@@ -3561,6 +3562,10 @@ static int hidpp_probe(struct hid_device *hdev, const 
struct hid_device_id *id)
ret = k400_allocate(hdev);
if (ret)
return ret;
+   } else if (hidpp->quirks & HIDPP_QUIRK_CLASS_G920) {
+   ret = g920_allocate(hdev);
+   if (ret)
+   return ret;
}
 
INIT_WORK(>work, delayed_work_cb);
-- 
2.21.0

[PATCH 2/3] HID: logitech-hidpp: split g920_get_config()

[Andrey Smirnov]

9k_hw 
coretemp snd_hda_codec_hdmi cordic kvm_intel snd_hda_codec_cirrus mac80211 
snd_hda_codec_generic ledtrig_audio kvm snd_hda_intel snd_intel_nhlt irqbypass 
snd_hda_codec btusb btrtl snd_hda_core ath btbcm ssb snd_hwdep btintel snd_seq 
crct10dif_pclmul iTCO_wdt snd_seq_device crc32_pclmul bluetooth mmc_core 
iTCO_vendor_support joydev cfg80211
[   88.320602]  applesmc ghash_clmulni_intel ecdh_generic snd_pcm input_polldev 
intel_cstate ecc intel_uncore thunderbolt snd_timer i2c_i801 libarc4 rfkill 
intel_rapl_perf lpc_ich mei_me pcspkr bcm5974 snd bcma mei soundcore acpi_als 
sbs kfifo_buf sbshc industrialio apple_bl i915 i2c_algo_bit drm_kms_helper drm 
uas crc32c_intel usb_storage video hid_apple
[   88.320630] CR2: 0018
[   88.320633] ---[ end trace 933491c8a4fadeb7 ]---
[   88.320642] RIP: 0010:hidpp_probe+0x61f/0x948 [hid_logitech_hidpp]
[   88.320645] Code: 81 00 00 48 89 ef e8 f0 d6 ff ff 41 89 c6 85 c0 75 b5 0f 
b6 44 24 28 48 8b 5d 00 88 44 24 1e 89 44 24 0c 48 8b 83 18 1c 00 00 <48> 8b 48 
18 48 8b 83 10 19 00 00 48 8b 40 40 48 89 0c 24 0f b7 80
[   88.320647] RSP: 0018:b0a6824aba68 EFLAGS: 00010246
[   88.320650] RAX:  RBX: 93a50756e000 RCX: 00010408
[   88.320652] RDX:  RSI: 93a51f0ad0a0 RDI: 0002d0a0
[   88.320655] RBP: 93a50416da28 R08: 93a50416da70 R09: 93a50416da70
[   88.320657] R10: 00148ae9e60c R11: 000f1525 R12: 93a50756e000
[   88.320659] R13: 93a50756f8d0 R14:  R15: 93a50756fc38
[   88.320662] FS:  7f8d8c1e0940() GS:93a51f08() 
knlGS:
[   88.320664] CS:  0010 DS:  ES:  CR0: 80050033
[   88.320667] CR2: 0018 CR3: 0003996d8003 CR4: 001606e0

To solve this issue:

   1. Split g920_get_config() such that all of the device specific
  communication remains a part of the function and input subsystem
  initialization bits go to hidpp_ff_init()

   2. Move call to hidpp_ff_init() from being a part of
  g920_get_config() to be the last step of .probe(), right after a
  call to hid_hw_start() with connect_mask containing
  HID_CONNECT_HIDINPUT.

Fixes: 91cf9a98ae41 ("HID: logitech-hidpp: make .probe usbhid capable")
Signed-off-by: Andrey Smirnov 
Cc: Jiri Kosina 
Cc: Benjamin Tissoires 
Cc: Henrik Rydberg 
Cc: Sam Bazely 
Cc: Pierre-Loup A. Griffais 
Cc: Austin Palmer 
Cc: linux-in...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: sta...@vger.kernel.org
---
 drivers/hid/hid-logitech-hidpp.c | 134 ---
 1 file changed, 85 insertions(+), 49 deletions(-)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 58eb928224e5..cadf36d6c6f3 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -1669,6 +1669,7 @@ static void hidpp_touchpad_raw_xy_event(struct 
hidpp_device *hidpp_dev,
 
 #define HIDPP_FF_EFFECTID_NONE -1
 #define HIDPP_FF_EFFECTID_AUTOCENTER   -2
+#define HIDPP_AUTOCENTER_PARAMS_LENGTH 18
 
 #define HIDPP_FF_MAX_PARAMS20
 #define HIDPP_FF_RESERVED_SLOTS1
@@ -2009,7 +2010,7 @@ static int hidpp_ff_erase_effect(struct input_dev *dev, 
int effect_id)
 static void hidpp_ff_set_autocenter(struct input_dev *dev, u16 magnitude)
 {
struct hidpp_ff_private_data *data = dev->ff->private;
-   u8 params[18];
+   u8 params[HIDPP_AUTOCENTER_PARAMS_LENGTH];
 
dbg_hid("Setting autocenter to %d.\n", magnitude);
 
@@ -2086,7 +2087,7 @@ static void hidpp_ff_destroy(struct ff_device *ff)
ff->private = NULL;
 }
 
-static int hidpp_ff_init(struct hidpp_device *hidpp, u8 feature_index)
+static int hidpp_ff_init(struct hidpp_device *hidpp)
 {
struct hid_device *hid = hidpp->hid_dev;
struct hid_input *hidinput = list_entry(hid->inputs.next, struct 
hid_input, list);
@@ -2094,9 +2095,8 @@ static int hidpp_ff_init(struct hidpp_device *hidpp, u8 
feature_index)
const struct usb_device_descriptor *udesc = 
&(hid_to_usb_dev(hid)->descriptor);
const u16 bcdDevice = le16_to_cpu(udesc->bcdDevice);
struct ff_device *ff;
-   struct hidpp_report response;
struct hidpp_ff_private_data *data = hidpp->private_data;
-   int error, j, num_slots;
+   int error, j, num_slots = data->num_effects;
u8 version;
 
if (!dev) {
@@ -2114,19 +2114,6 @@ static int hidpp_ff_init(struct hidpp_device *hidpp, u8 
feature_index)
for (j = 0; hidpp_ff_effects_v2[j] >= 0; j++)
set_bit(hidpp_ff_effects_v2[j], dev->ffbit);
 
-   /* Read number of slots available in device */
-   error = hidpp_send_fap_command_sync(hidpp, feature_index,
-   HIDPP_FF_GET_INFO, NULL, 0, );
-   if (error) {
-   if (error < 0)
-   return error;
-   hid_err(hidpp->hid_

[PATCH 0/3] Logitech G920 fixes

[Andrey Smirnov]

Everyone:

This series contains patches to fix a couple of regressions in G920
wheel support by hid-logitech-hidpp driver. Without the patches the
wheel remains stuck in autocentering mode ("resisting" any attempt to
trun) as well as missing support for any FF action.

Thanks,
Andrey Smirnov

Andrey Smirnov (3):
  HID: logitech-hidpp: use devres to manage FF private data
  HID: logitech-hidpp: split g920_get_config()
  HID: logitech-hidpp: add G920 device validation quirk

 drivers/hid/hid-logitech-hidpp.c | 193 +++
 1 file changed, 120 insertions(+), 73 deletions(-)

-- 
2.21.0

[PATCH] dma-mapping: fix false positivse warnings in dma_common_free_remap()

[Andrey Smirnov]

Commit 5cf4537975bb ("dma-mapping: introduce a dma_common_find_pages
helper") changed invalid input check in dma_common_free_remap() from:

if (!area || !area->flags != VM_DMA_COHERENT)

to

if (!area || !area->flags != VM_DMA_COHERENT || !area->pages)

which seem to produce false positives for memory obtained via
dma_common_contiguous_remap()

This triggers the following warning message when doing "reboot" on ZII
VF610 Dev Board Rev B:

WARNING: CPU: 0 PID: 1 at kernel/dma/remap.c:112 dma_common_free_remap+0x88/0x8c
trying to free invalid coherent area: 9ef82980
Modules linked in:
CPU: 0 PID: 1 Comm: systemd-shutdow Not tainted 5.3.0-rc6-next-20190820 #119
Hardware name: Freescale Vybrid VF5xx/VF6xx (Device Tree)
Backtrace:
[<8010d1ec>] (dump_backtrace) from [<8010d588>] (show_stack+0x20/0x24)
 r7:8015ed78 r6:0009 r5: r4:9f4d9b14
[<8010d568>] (show_stack) from [<8077e3f0>] (dump_stack+0x24/0x28)
[<8077e3cc>] (dump_stack) from [<801197a0>] (__warn.part.3+0xcc/0xe4)
[<801196d4>] (__warn.part.3) from [<80119830>] (warn_slowpath_fmt+0x78/0x94)
 r6:0070 r5:808e540c r4:81c03048
[<801197bc>] (warn_slowpath_fmt) from [<8015ed78>] 
(dma_common_free_remap+0x88/0x8c)
 r3:9ef82980 r2:808e53e0
 r7:1000 r6:a0b1e000 r5:a0b1e000 r4:1000
[<8015ecf0>] (dma_common_free_remap) from [<8010fa9c>] 
(remap_allocator_free+0x60/0x68)
 r5:81c03048 r4:9f4d9b78
[<8010fa3c>] (remap_allocator_free) from [<801100d0>] 
(__arm_dma_free.constprop.3+0xf8/0x148)
 r5:81c03048 r4:9ef82900
[<8010ffd8>] (__arm_dma_free.constprop.3) from [<80110144>] 
(arm_dma_free+0x24/0x2c)
 r5:9f563410 r4:80110120
[<80110120>] (arm_dma_free) from [<8015d80c>] (dma_free_attrs+0xa0/0xdc)
[<8015d76c>] (dma_free_attrs) from [<8020f3e4>] (dma_pool_destroy+0xc0/0x154)
 r8:9efa8860 r7:808f02f0 r6:808f02d0 r5:9ef82880 r4:9ef82780
[<8020f324>] (dma_pool_destroy) from [<805525d0>] (ehci_mem_cleanup+0x6c/0x150)
 r7:9f563410 r6:9efa8810 r5: r4:9efd0148
[<80552564>] (ehci_mem_cleanup) from [<80558e0c>] (ehci_stop+0xac/0xc0)
 r5:9efd0148 r4:9efd
[<80558d60>] (ehci_stop) from [<8053c4bc>] (usb_remove_hcd+0xf4/0x1b0)
 r7:9f563410 r6:9efd0074 r5:81c03048 r4:9efd
[<8053c3c8>] (usb_remove_hcd) from [<8056361c>] (host_stop+0x48/0xb8)
 r7:9f563410 r6:9efd r5:9f5f4040 r4:9f5f5040
[<805635d4>] (host_stop) from [<80563d0c>] (ci_hdrc_host_destroy+0x34/0x38)
 r7:9f563410 r6:9f5f5040 r5:9efa8800 r4:9f5f4040
[<80563cd8>] (ci_hdrc_host_destroy) from [<8055ef18>] 
(ci_hdrc_remove+0x50/0x10c)
[<8055eec8>] (ci_hdrc_remove) from [<804a2ed8>] (platform_drv_remove+0x34/0x4c)
 r7:9f563410 r6:81c4f99c r5:9efa8810 r4:9efa8810
[<804a2ea4>] (platform_drv_remove) from [<804a18a8>] 
(device_release_driver_internal+0xec/0x19c)
 r5: r4:9efa8810
[<804a17bc>] (device_release_driver_internal) from [<804a1978>] 
(device_release_driver+0x20/0x24)
 r7:9f563410 r6:81c41ed0 r5:9efa8810 r4:9f4a1dac
[<804a1958>] (device_release_driver) from [<804a01b8>] 
(bus_remove_device+0xdc/0x108)
[<804a00dc>] (bus_remove_device) from [<8049c204>] (device_del+0x150/0x36c)
 r7:9f563410 r6:81c03048 r5:9efa8854 r4:9efa8810
[<8049c0b4>] (device_del) from [<804a3368>] 
(platform_device_del.part.2+0x20/0x84)
 r10:9f563414 r9:809177e0 r8:81cb07dc r7:81c78320 r6:9f563454 r5:9efa8800
 r4:9efa8800
[<804a3348>] (platform_device_del.part.2) from [<804a3420>] 
(platform_device_unregister+0x28/0x34)
 r5:9f563400 r4:9efa8800
[<804a33f8>] (platform_device_unregister) from [<8055dce0>] 
(ci_hdrc_remove_device+0x1c/0x30)
 r5:9f563400 r4:0001
[<8055dcc4>] (ci_hdrc_remove_device) from [<805652ac>] 
(ci_hdrc_imx_remove+0x38/0x118)
 r7:81c78320 r6:9f563454 r5:9f563410 r4:9f541010
[<8056538c>] (ci_hdrc_imx_shutdown) from [<804a2970>] 
(platform_drv_shutdown+0x2c/0x30)
[<804a2944>] (platform_drv_shutdown) from [<8049e4fc>] 
(device_shutdown+0x158/0x1f0)
[<8049e3a4>] (device_shutdown) from [<8013ac80>] 
(kernel_restart_prepare+0x44/0x48)
 r10:0058 r9:9f4d8000 r8:fee1dead r7:379ce700 r6:81c0b280 r5:81c03048
 r4:
[<8013ac3c>] (kernel_restart_prepare) from [<8013ad14>] 
(kernel_restart+0x1c/0x60)
[<8013acf8>] (kernel_restart) from [<8013af84>] (__do_sys_reboot+0xe0/0x1d8)
 r5:81c03048 r4:
[<8013aea4>] (__do_sys_reboot) from [<8013b0ec>] (sys_reboot+0x18/0x1c)
 r8:80101204 r7:0058 r6: r5: r4:
[<8013b0d4>] (sys_reboot) from [<80101000>] (ret_fast_syscall+0x0/0x54)
Exception stack(0x9f4d9fa8 to 0x9f4d9ff0)
9fa0:     fee1dead 28121969 01234567 379ce700
9fc0:    0058  0

[PATCH] tty: serial: fsl_lpuart: Fix lpuart_flush_buffer()

[Andrey Smirnov]

Fix incorrect read-modify-write sequence in lpuart_flush_buffer() that
was reading from UARTPFIFO and writing to UARTCFIFO instead of
operating solely on the latter.

Fixes: 9bc19af9dacb ("tty: serial: fsl_lpuart: Flush HW FIFOs in .flush_buffer")
Signed-off-by: Andrey Smirnov 
Cc: Stefan Agner 
Cc: Andrew Lunn 
Cc: Vivien Didelot 
Cc: Chris Healy 
Cc: Cory Tusar 
Cc: Lucas Stach 
Cc: Greg Kroah-Hartman 
Cc: Jiri Slaby 
Cc: linux-...@nxp.com
Cc: linux-ser...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---

Greg:

This bug causes occasional boot hang on 5.4-rc1 on Vybrid, so it might
be good to push that for 5.4-rc2.

Thanks,
Andrey Sirnov

 drivers/tty/serial/fsl_lpuart.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
index 3e17bb8a0b16..537896c4d887 100644
--- a/drivers/tty/serial/fsl_lpuart.c
+++ b/drivers/tty/serial/fsl_lpuart.c
@@ -548,7 +548,7 @@ static void lpuart_flush_buffer(struct uart_port *port)
val |= UARTFIFO_TXFLUSH | UARTFIFO_RXFLUSH;
lpuart32_write(>port, val, UARTFIFO);
} else {
-   val = readb(sport->port.membase + UARTPFIFO);
+   val = readb(sport->port.membase + UARTCFIFO);
val |= UARTCFIFO_TXFLUSH | UARTCFIFO_RXFLUSH;
writeb(val, sport->port.membase + UARTCFIFO);
}
-- 
2.21.0

[PATCH] ARM: dts: vf610-zii-scu4-aib: Specify 'i2c-mux-idle-disconnect'

[Andrey Smirnov]

Specify 'i2c-mux-idle-disconnect' for both I2C switches present on the
board, since both are connected to the same parent bus and all of
their children have the same I2C address.

Fixes: ca4b4d373fcc ("ARM: dts: vf610: Add ZII SCU4 AIB board")
Signed-off-by: Andrey Smirnov 
Cc: Shawn Guo 
Cc: Chris Healy 
Cc: Cory Tusar 
Cc: Jeff White 
Cc: Rick Ramstetter 
Cc: Lucas Stach 
Cc: Fabio Estevam 
Cc: linux-arm-ker...@lists.infradead.org
Cc: devicet...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
Shawn:

If this is possible, I'd like this one to go into 5.4.

Thanks,
Andrey Smirnov

 arch/arm/boot/dts/vf610-zii-scu4-aib.dts | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/boot/dts/vf610-zii-scu4-aib.dts 
b/arch/arm/boot/dts/vf610-zii-scu4-aib.dts
index dc8a5f37a1ef..c8ebb23c4e02 100644
--- a/arch/arm/boot/dts/vf610-zii-scu4-aib.dts
+++ b/arch/arm/boot/dts/vf610-zii-scu4-aib.dts
@@ -602,6 +602,7 @@
#address-cells = <1>;
#size-cells = <0>;
reg = <0x70>;
+   i2c-mux-idle-disconnect;
 
sff0_i2c: i2c@1 {
#address-cells = <1>;
@@ -640,6 +641,7 @@
reg = <0x71>;
#address-cells = <1>;
#size-cells = <0>;
+   i2c-mux-idle-disconnect;
 
sff5_i2c: i2c@1 {
#address-cells = <1>;
-- 
2.21.0

[PATCH] ARM: dts: am3874-iceboard: Fix 'i2c-mux-idle-disconnect' usage

[Andrey Smirnov]

According to
Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt,
i2c-mux-idle-disconnect is a property of a parent node since it
pertains to the mux/switch as a whole, so move it there and drop all
of the concurrences in child nodes.

Fixes: d031773169df ("ARM: dts: Adds device tree file for McGill's IceBoard, 
based on TI AM3874")
Signed-off-by: Andrey Smirnov 
Cc: Benoît Cousson 
Cc: Tony Lindgren 
Cc: Graeme Smecher 
Cc: linux-o...@vger.kernel.org
Cc: devicet...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---

This is purely a drive-by fix, since it concerns the HW I've never
heard of before. However I was working with PCA9548
(vf610-zii-scu4-aib is my HW) and looking at various users in the
kernel, when this code caught my eye. Apologies for the noise if this
fix is somehow bogus.

In case that it matters this patch is based on top of 5.4-rc1.

Thanks,
Andrey Smirnov

 arch/arm/boot/dts/am3874-iceboard.dts | 9 +
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/arch/arm/boot/dts/am3874-iceboard.dts 
b/arch/arm/boot/dts/am3874-iceboard.dts
index 883fb85135d4..1b4b2b0500e4 100644
--- a/arch/arm/boot/dts/am3874-iceboard.dts
+++ b/arch/arm/boot/dts/am3874-iceboard.dts
@@ -111,13 +111,13 @@
reg = <0x70>;
#address-cells = <1>;
#size-cells = <0>;
+   i2c-mux-idle-disconnect;
 
i2c@0 {
/* FMC A */
#address-cells = <1>;
#size-cells = <0>;
reg = <0>;
-   i2c-mux-idle-disconnect;
};
 
i2c@1 {
@@ -125,7 +125,6 @@
#address-cells = <1>;
#size-cells = <0>;
reg = <1>;
-   i2c-mux-idle-disconnect;
};
 
i2c@2 {
@@ -133,7 +132,6 @@
#address-cells = <1>;
#size-cells = <0>;
reg = <2>;
-   i2c-mux-idle-disconnect;
};
 
i2c@3 {
@@ -141,7 +139,6 @@
#address-cells = <1>;
#size-cells = <0>;
reg = <3>;
-   i2c-mux-idle-disconnect;
};
 
i2c@4 {
@@ -149,14 +146,12 @@
#address-cells = <1>;
#size-cells = <0>;
reg = <4>;
-   i2c-mux-idle-disconnect;
};
 
i2c@5 {
#address-cells = <1>;
#size-cells = <0>;
reg = <5>;
-   i2c-mux-idle-disconnect;
 
ina230@40 { compatible = "ti,ina230"; reg = <0x40>; 
shunt-resistor = <5000>; };
ina230@41 { compatible = "ti,ina230"; reg = <0x41>; 
shunt-resistor = <5000>; };
@@ -182,14 +177,12 @@
#address-cells = <1>;
#size-cells = <0>;
reg = <6>;
-   i2c-mux-idle-disconnect;
};
 
i2c@7 {
#address-cells = <1>;
#size-cells = <0>;
reg = <7>;
-   i2c-mux-idle-disconnect;
 
u41: pca9575@20 {
compatible = "nxp,pca9575";
-- 
2.21.0

Re: [PATCH] Bluetooth: hidp: Fix assumptions on the return value of hidp_send_message

[Andrey Smirnov]

> I am taking this through my tree. And yes, I applied the updated patch, but 
> answered the other ;)
>
> Regards
>
> Marcel

Are you also going to mark this patch for inclusion to stable tree? I haven't 
seen it in any of the stable queues (netdev, Greg's), so just wanted to check.

Thanks,
Andrey Smirnov

[PATCH v7 10/12] thermal: qoriq: Do not report invalid temperature reading

[Andrey Smirnov]

Before returning measured temperature data to upper layer we need to
make sure that the reading was marked as "valid" to avoid reporting
bogus data.

Signed-off-by: Andrey Smirnov 
Reviewed-by: Daniel Lezcano 
Tested-by: Lucas Stach 
Cc: Chris Healy 
Cc: Lucas Stach 
Cc: Eduardo Valentin 
Cc: Daniel Lezcano 
Cc: Angus Ainslie (Purism) 
Cc: linux-...@nxp.com
Cc: linux...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/thermal/qoriq_thermal.c | 19 ++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c
index 1cc53a4a5c47..48853192514a 100644
--- a/drivers/thermal/qoriq_thermal.c
+++ b/drivers/thermal/qoriq_thermal.c
@@ -38,6 +38,7 @@
 #define REGS_TRITSR(n) (0x100 + 16 * (n)) /* Immediate Temperature
* Site Register
*/
+#define TRITSR_V   BIT(31)
 #define REGS_TTRnCR(n) (0xf10 + 4 * (n)) /* Temperature Range n
   * Control Register
   */
@@ -64,8 +65,24 @@ static int tmu_get_temp(void *p, int *temp)
struct qoriq_sensor *qsensor = p;
struct qoriq_tmu_data *qdata = qoriq_sensor_to_data(qsensor);
u32 val;
+   /*
+* REGS_TRITSR(id) has the following layout:
+*
+* 31  ... 7 6 5 4 3 2 1 0
+*  V  TEMP
+*
+* Where V bit signifies if the measurement is ready and is
+* within sensor range. TEMP is an 8 bit value representing
+* temperature in C.
+*/
+   if (regmap_read_poll_timeout(qdata->regmap,
+REGS_TRITSR(qsensor->id),
+val,
+val & TRITSR_V,
+USEC_PER_MSEC,
+10 * USEC_PER_MSEC))
+   return -ENODATA;
 
-   regmap_read(qdata->regmap, REGS_TRITSR(qsensor->id), );
*temp = (val & 0xff) * 1000;
 
return 0;
-- 
2.21.0

[PATCH v7 11/12] thermal_hwmon: Add devres wrapper for thermal_add_hwmon_sysfs()

[Andrey Smirnov]

Add devres wrapper for thermal_add_hwmon_sysfs() to simplify driver
code.

Signed-off-by: Andrey Smirnov 
Reviewed-by: Daniel Lezcano 
Tested-by: Lucas Stach 
Cc: Chris Healy 
Cc: Lucas Stach 
Cc: Eduardo Valentin 
Cc: Daniel Lezcano 
Cc: Angus Ainslie (Purism) 
Cc: linux-...@nxp.com
Cc: linux...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/thermal/thermal_hwmon.c | 28 
 drivers/thermal/thermal_hwmon.h |  7 +++
 2 files changed, 35 insertions(+)

diff --git a/drivers/thermal/thermal_hwmon.c b/drivers/thermal/thermal_hwmon.c
index dd5d8ee37928..c8d2620f2e42 100644
--- a/drivers/thermal/thermal_hwmon.c
+++ b/drivers/thermal/thermal_hwmon.c
@@ -248,3 +248,31 @@ void thermal_remove_hwmon_sysfs(struct thermal_zone_device 
*tz)
kfree(hwmon);
 }
 EXPORT_SYMBOL_GPL(thermal_remove_hwmon_sysfs);
+
+static void devm_thermal_hwmon_release(struct device *dev, void *res)
+{
+   thermal_remove_hwmon_sysfs(*(struct thermal_zone_device **)res);
+}
+
+int devm_thermal_add_hwmon_sysfs(struct thermal_zone_device *tz)
+{
+   struct thermal_zone_device **ptr;
+   int ret;
+
+   ptr = devres_alloc(devm_thermal_hwmon_release, sizeof(*ptr),
+  GFP_KERNEL);
+   if (!ptr)
+   return -ENOMEM;
+
+   ret = thermal_add_hwmon_sysfs(tz);
+   if (ret) {
+   devres_free(ptr);
+   return ret;
+   }
+
+   *ptr = tz;
+   devres_add(>device, ptr);
+
+   return ret;
+}
+EXPORT_SYMBOL_GPL(devm_thermal_add_hwmon_sysfs);
diff --git a/drivers/thermal/thermal_hwmon.h b/drivers/thermal/thermal_hwmon.h
index a160b9d62dd0..1a9d65f6a6a8 100644
--- a/drivers/thermal/thermal_hwmon.h
+++ b/drivers/thermal/thermal_hwmon.h
@@ -17,6 +17,7 @@
 
 #ifdef CONFIG_THERMAL_HWMON
 int thermal_add_hwmon_sysfs(struct thermal_zone_device *tz);
+int devm_thermal_add_hwmon_sysfs(struct thermal_zone_device *tz);
 void thermal_remove_hwmon_sysfs(struct thermal_zone_device *tz);
 #else
 static inline int
@@ -25,6 +26,12 @@ thermal_add_hwmon_sysfs(struct thermal_zone_device *tz)
return 0;
 }
 
+static inline int
+devm_thermal_add_hwmon_sysfs(struct thermal_zone_device *tz)
+{
+   return 0;
+}
+
 static inline void
 thermal_remove_hwmon_sysfs(struct thermal_zone_device *tz)
 {
-- 
2.21.0

[PATCH v7 08/12] thermal: qoriq: Convert driver to use regmap API

[Andrey Smirnov]

Convert driver to use regmap API, drop custom LE/BE IO helpers and
simplify bit manipulation using regmap_update_bits(). This also allows
us to convert some register initialization to use loops and adds
convenient debug access to TMU registers via debugfs.

Signed-off-by: Andrey Smirnov 
Reviewed-by: Daniel Lezcano 
Tested-by: Lucas Stach 
Cc: Chris Healy 
Cc: Lucas Stach 
Cc: Eduardo Valentin 
Cc: Daniel Lezcano 
Cc: Angus Ainslie (Purism) 
Cc: linux-...@nxp.com
Cc: linux...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/thermal/qoriq_thermal.c | 153 +++-
 1 file changed, 72 insertions(+), 81 deletions(-)

diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c
index 8a28a4433d44..32bf5ed57f5c 100644
--- a/drivers/thermal/qoriq_thermal.c
+++ b/drivers/thermal/qoriq_thermal.c
@@ -9,6 +9,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include "thermal_core.h"
@@ -18,48 +19,27 @@
 /*
  * QorIQ TMU Registers
  */
-struct qoriq_tmu_site_regs {
-   u32 tritsr; /* Immediate Temperature Site Register */
-   u32 tratsr; /* Average Temperature Site Register */
-   u8 res0[0x8];
-};
 
-struct qoriq_tmu_regs {
-   u32 tmr;/* Mode Register */
+#define REGS_TMR   0x000   /* Mode Register */
 #define TMR_DISABLE0x0
 #define TMR_ME 0x8000
 #define TMR_ALPF   0x0c00
-   u32 tsr;/* Status Register */
-   u32 tmtmir; /* Temperature measurement interval Register */
+
+#define REGS_TMTMIR0x008   /* Temperature measurement interval Register */
 #define TMTMIR_DEFAULT 0x000f
-   u8 res0[0x14];
-   u32 tier;   /* Interrupt Enable Register */
+
+#define REGS_TIER  0x020   /* Interrupt Enable Register */
 #define TIER_DISABLE   0x0
-   u32 tidr;   /* Interrupt Detect Register */
-   u32 tiscr;  /* Interrupt Site Capture Register */
-   u32 ticscr; /* Interrupt Critical Site Capture Register */
-   u8 res1[0x10];
-   u32 tmhtcrh;/* High Temperature Capture Register */
-   u32 tmhtcrl;/* Low Temperature Capture Register */
-   u8 res2[0x8];
-   u32 tmhtitr;/* High Temperature Immediate Threshold */
-   u32 tmhtatr;/* High Temperature Average Threshold */
-   u32 tmhtactr;   /* High Temperature Average Crit Threshold */
-   u8 res3[0x24];
-   u32 ttcfgr; /* Temperature Configuration Register */
-   u32 tscfgr; /* Sensor Configuration Register */
-   u8 res4[0x78];
-   struct qoriq_tmu_site_regs site[SITES_MAX];
-   u8 res5[0x9f8];
-   u32 ipbrr0; /* IP Block Revision Register 0 */
-   u32 ipbrr1; /* IP Block Revision Register 1 */
-   u8 res6[0x310];
-   u32 ttr0cr; /* Temperature Range 0 Control Register */
-   u32 ttr1cr; /* Temperature Range 1 Control Register */
-   u32 ttr2cr; /* Temperature Range 2 Control Register */
-   u32 ttr3cr; /* Temperature Range 3 Control Register */
-};
 
+#define REGS_TTCFGR0x080   /* Temperature Configuration Register */
+#define REGS_TSCFGR0x084   /* Sensor Configuration Register */
+
+#define REGS_TRITSR(n) (0x100 + 16 * (n)) /* Immediate Temperature
+   * Site Register
+   */
+#define REGS_TTRnCR(n) (0xf10 + 4 * (n)) /* Temperature Range n
+  * Control Register
+  */
 /*
  * Thermal zone data
  */
@@ -68,9 +48,8 @@ struct qoriq_sensor {
 };
 
 struct qoriq_tmu_data {
-   struct qoriq_tmu_regs __iomem *regs;
+   struct regmap *regmap;
struct clk *clk;
-   bool little_endian;
struct qoriq_sensor sensor[SITES_MAX];
 };
 
@@ -79,29 +58,13 @@ static struct qoriq_tmu_data *qoriq_sensor_to_data(struct 
qoriq_sensor *s)
return container_of(s, struct qoriq_tmu_data, sensor[s->id]);
 }
 
-static void tmu_write(struct qoriq_tmu_data *p, u32 val, void __iomem *addr)
-{
-   if (p->little_endian)
-   iowrite32(val, addr);
-   else
-   iowrite32be(val, addr);
-}
-
-static u32 tmu_read(struct qoriq_tmu_data *p, void __iomem *addr)
-{
-   if (p->little_endian)
-   return ioread32(addr);
-   else
-   return ioread32be(addr);
-}
-
 static int tmu_get_temp(void *p, int *temp)
 {
struct qoriq_sensor *qsensor = p;
struct qoriq_tmu_data *qdata = qoriq_sensor_to_data(qsensor);
u32 val;
 
-   val = tmu_read(qdata, >regs->site[qsensor->id].tritsr);
+   regmap_read(qdata->regmap, REGS_TRITSR(qsensor->id), );
*temp = (val & 0xff) * 1000;
 
return 0;
@@ -139,7 +102,8 @@ 

[PATCH v7 09/12] thermal: qoriq: Enable all sensors before registering them

[Andrey Smirnov]

Tmu_get_temp will get called as a part of sensor registration via
devm_thermal_zone_of_sensor_register(). To prevent it from retruning
bogus data we need to enable sensor monitoring before that. Looking at
the datasheet (i.MX8MQ RM) there doesn't seem to be any harm in
enabling them all, so, for the sake of simplicity, change the code to
do just that.

Signed-off-by: Andrey Smirnov 
Tested-by: Lucas Stach 
Cc: Chris Healy 
Cc: Lucas Stach 
Cc: Eduardo Valentin 
Cc: Daniel Lezcano 
Cc: Angus Ainslie (Purism) 
Cc: linux-...@nxp.com
Cc: linux...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/thermal/qoriq_thermal.c | 18 --
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c
index 32bf5ed57f5c..1cc53a4a5c47 100644
--- a/drivers/thermal/qoriq_thermal.c
+++ b/drivers/thermal/qoriq_thermal.c
@@ -24,6 +24,7 @@
 #define TMR_DISABLE0x0
 #define TMR_ME 0x8000
 #define TMR_ALPF   0x0c00
+#define TMR_MSITE_ALL  GENMASK(15, 0)
 
 #define REGS_TMTMIR0x008   /* Temperature measurement interval Register */
 #define TMTMIR_DEFAULT 0x000f
@@ -77,7 +78,10 @@ static const struct thermal_zone_of_device_ops tmu_tz_ops = {
 static int qoriq_tmu_register_tmu_zone(struct device *dev,
   struct qoriq_tmu_data *qdata)
 {
-   int id, sites = 0;
+   int id;
+
+   regmap_write(qdata->regmap, REGS_TMR,
+TMR_MSITE_ALL | TMR_ME | TMR_ALPF);
 
for (id = 0; id < SITES_MAX; id++) {
struct thermal_zone_device *tzd;
@@ -93,18 +97,12 @@ static int qoriq_tmu_register_tmu_zone(struct device *dev,
if (ret) {
if (ret == -ENODEV)
continue;
-   else
-   return ret;
-   }
 
-   sites |= 0x1 << (15 - id);
+   regmap_write(qdata->regmap, REGS_TMR, TMR_DISABLE);
+   return ret;
+   }
}
 
-   /* Enable monitoring */
-   if (sites != 0)
-   regmap_write(qdata->regmap, REGS_TMR,
-sites | TMR_ME | TMR_ALPF);
-
return 0;
 }
 
-- 
2.21.0

[PATCH v7 12/12] thermal: qoriq: Add hwmon support

[Andrey Smirnov]

Expose thermal readings as a HWMON device, so that it could be
accessed using lm-sensors.

Signed-off-by: Andrey Smirnov 
Reviewed-by: Daniel Lezcano 
Tested-by: Lucas Stach 
Cc: Chris Healy 
Cc: Lucas Stach 
Cc: Eduardo Valentin 
Cc: Daniel Lezcano 
Cc: Angus Ainslie (Purism) 
Cc: linux-...@nxp.com
Cc: linux...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/thermal/qoriq_thermal.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c
index 48853192514a..e907f0d2103f 100644
--- a/drivers/thermal/qoriq_thermal.c
+++ b/drivers/thermal/qoriq_thermal.c
@@ -13,6 +13,7 @@
 #include 
 
 #include "thermal_core.h"
+#include "thermal_hwmon.h"
 
 #define SITES_MAX  16
 
@@ -118,6 +119,11 @@ static int qoriq_tmu_register_tmu_zone(struct device *dev,
regmap_write(qdata->regmap, REGS_TMR, TMR_DISABLE);
return ret;
}
+
+   if (devm_thermal_add_hwmon_sysfs(tzd))
+   dev_warn(dev,
+"Failed to add hwmon sysfs attributes\n");
+
}
 
return 0;
-- 
2.21.0

[PATCH v7 07/12] thermal: qoriq: Drop unnecessary drvdata cleanup

[Andrey Smirnov]

Driver data of underlying struct device will be set to NULL by Linux's
driver infrastructure. Clearing it here is unnecessary.

Signed-off-by: Andrey Smirnov 
Cc: Chris Healy 
Cc: Lucas Stach 
Cc: Eduardo Valentin 
Cc: Daniel Lezcano 
Cc: Angus Ainslie (Purism) 
Cc: linux-...@nxp.com
Cc: linux...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/thermal/qoriq_thermal.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c
index af596c3342d0..8a28a4433d44 100644
--- a/drivers/thermal/qoriq_thermal.c
+++ b/drivers/thermal/qoriq_thermal.c
@@ -253,8 +253,6 @@ static int qoriq_tmu_remove(struct platform_device *pdev)
 
clk_disable_unprepare(data->clk);
 
-   platform_set_drvdata(pdev, NULL);
-
return 0;
 }
 
-- 
2.21.0

[PATCH v7 01/12] thermal: qoriq: Add local struct device pointer

[Andrey Smirnov]

Use a local "struct device *dev" for brevity. No functional change
intended.

Signed-off-by: Andrey Smirnov 
Acked-by: Daniel Lezcano 
Tested-by: Lucas Stach 
Cc: Chris Healy 
Cc: Lucas Stach 
Cc: Eduardo Valentin 
Cc: Daniel Lezcano 
Cc: Angus Ainslie (Purism) 
Cc: linux-...@nxp.com
Cc: linux...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/thermal/qoriq_thermal.c | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c
index 39542c670301..5df6267a5da0 100644
--- a/drivers/thermal/qoriq_thermal.c
+++ b/drivers/thermal/qoriq_thermal.c
@@ -194,8 +194,9 @@ static int qoriq_tmu_probe(struct platform_device *pdev)
int ret;
struct qoriq_tmu_data *data;
struct device_node *np = pdev->dev.of_node;
+   struct device *dev = >dev;
 
-   data = devm_kzalloc(>dev, sizeof(struct qoriq_tmu_data),
+   data = devm_kzalloc(dev, sizeof(struct qoriq_tmu_data),
GFP_KERNEL);
if (!data)
return -ENOMEM;
@@ -206,17 +207,17 @@ static int qoriq_tmu_probe(struct platform_device *pdev)
 
data->regs = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(data->regs)) {
-   dev_err(>dev, "Failed to get memory region\n");
+   dev_err(dev, "Failed to get memory region\n");
return PTR_ERR(data->regs);
}
 
-   data->clk = devm_clk_get_optional(>dev, NULL);
+   data->clk = devm_clk_get_optional(dev, NULL);
if (IS_ERR(data->clk))
return PTR_ERR(data->clk);
 
ret = clk_prepare_enable(data->clk);
if (ret) {
-   dev_err(>dev, "Failed to enable clock\n");
+   dev_err(dev, "Failed to enable clock\n");
return ret;
}
 
@@ -228,7 +229,7 @@ static int qoriq_tmu_probe(struct platform_device *pdev)
 
ret = qoriq_tmu_register_tmu_zone(pdev);
if (ret < 0) {
-   dev_err(>dev, "Failed to register sensors\n");
+   dev_err(dev, "Failed to register sensors\n");
ret = -ENODEV;
goto err;
}
-- 
2.21.0

[PATCH v7 02/12] thermal: qoriq: Don't store struct thermal_zone_device reference

[Andrey Smirnov]

Struct thermal_zone_device reference stored as sensor's private data
isn't really used anywhere in the code. Drop it.

Signed-off-by: Andrey Smirnov 
Acked-by: Daniel Lezcano 
Tested-by: Lucas Stach 
Cc: Chris Healy 
Cc: Lucas Stach 
Cc: Eduardo Valentin 
Cc: Daniel Lezcano 
Cc: Angus Ainslie (Purism) 
Cc: linux-...@nxp.com
Cc: linux...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/thermal/qoriq_thermal.c | 17 +++--
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c
index 5df6267a5da0..b471c226f06b 100644
--- a/drivers/thermal/qoriq_thermal.c
+++ b/drivers/thermal/qoriq_thermal.c
@@ -66,7 +66,6 @@ struct qoriq_tmu_data;
  * Thermal zone data
  */
 struct qoriq_sensor {
-   struct thermal_zone_device  *tzd;
struct qoriq_tmu_data   *qdata;
int id;
 };
@@ -116,6 +115,9 @@ static int qoriq_tmu_register_tmu_zone(struct 
platform_device *pdev)
int id, sites = 0;
 
for (id = 0; id < SITES_MAX; id++) {
+   struct thermal_zone_device *tzd;
+   int ret;
+
qdata->sensor[id] = devm_kzalloc(>dev,
sizeof(struct qoriq_sensor), GFP_KERNEL);
if (!qdata->sensor[id])
@@ -123,13 +125,16 @@ static int qoriq_tmu_register_tmu_zone(struct 
platform_device *pdev)
 
qdata->sensor[id]->id = id;
qdata->sensor[id]->qdata = qdata;
-   qdata->sensor[id]->tzd = devm_thermal_zone_of_sensor_register(
-   >dev, id, qdata->sensor[id], _tz_ops);
-   if (IS_ERR(qdata->sensor[id]->tzd)) {
-   if (PTR_ERR(qdata->sensor[id]->tzd) == -ENODEV)
+
+   tzd = devm_thermal_zone_of_sensor_register(>dev, id,
+  qdata->sensor[id],
+  _tz_ops);
+   ret = PTR_ERR_OR_ZERO(tzd);
+   if (ret) {
+   if (ret == -ENODEV)
continue;
else
-   return PTR_ERR(qdata->sensor[id]->tzd);
+   return ret;
}
 
sites |= 0x1 << (15 - id);
-- 
2.21.0

[PATCH v7 05/12] thermal: qoriq: Pass data to qoriq_tmu_register_tmu_zone() directly

[Andrey Smirnov]

Pass all necessary data to qoriq_tmu_register_tmu_zone() directly
instead of passing a platform device and then deriving it. This is
done as a first step to simplify resource deallocation code.

Signed-off-by: Andrey Smirnov 
Acked-by: Daniel Lezcano 
Tested-by: Lucas Stach 
Cc: Chris Healy 
Cc: Lucas Stach 
Cc: Eduardo Valentin 
Cc: Daniel Lezcano 
Cc: Angus Ainslie (Purism) 
Cc: linux-...@nxp.com
Cc: linux...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/thermal/qoriq_thermal.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c
index f8f5228d83af..5b9f2a31d275 100644
--- a/drivers/thermal/qoriq_thermal.c
+++ b/drivers/thermal/qoriq_thermal.c
@@ -111,9 +111,9 @@ static const struct thermal_zone_of_device_ops tmu_tz_ops = 
{
.get_temp = tmu_get_temp,
 };
 
-static int qoriq_tmu_register_tmu_zone(struct platform_device *pdev)
+static int qoriq_tmu_register_tmu_zone(struct device *dev,
+  struct qoriq_tmu_data *qdata)
 {
-   struct qoriq_tmu_data *qdata = platform_get_drvdata(pdev);
int id, sites = 0;
 
for (id = 0; id < SITES_MAX; id++) {
@@ -123,7 +123,7 @@ static int qoriq_tmu_register_tmu_zone(struct 
platform_device *pdev)
 
sensor->id = id;
 
-   tzd = devm_thermal_zone_of_sensor_register(>dev, id,
+   tzd = devm_thermal_zone_of_sensor_register(dev, id,
   sensor,
   _tz_ops);
ret = PTR_ERR_OR_ZERO(tzd);
@@ -229,7 +229,7 @@ static int qoriq_tmu_probe(struct platform_device *pdev)
if (ret < 0)
goto err;
 
-   ret = qoriq_tmu_register_tmu_zone(pdev);
+   ret = qoriq_tmu_register_tmu_zone(dev, data);
if (ret < 0) {
dev_err(dev, "Failed to register sensors\n");
ret = -ENODEV;
-- 
2.21.0

[PATCH v7 00/12] QorIQ TMU multi-sensor and HWMON support

[Andrey Smirnov]

Everyone:

This series contains patches adding support for HWMON integration, bug
fixes and general improvements (hopefully) for TMU driver I made while
working on it on i.MX8MQ.

Feedback is welcome!

Thanks,
Andrey Smirnov

Changes since [v6]:

   - Rebased on top of Zhang's "next" branch

   - Added "thermal: qoriq: Drop unnecessary drvdata cleanup"

Changes since [v5]

- Rebased on recent linux-next, dropped "thermal: qoriq: Remove
  unnecessary DT node is NULL check" since it is already in the
  tree

- Dropped dependency on [rfc]

Changes since [v4]

- Collected Tested-by from Lucas

- Collected Reviewed-by from Daniel

- Converted "thermal: qoriq: Enable all sensors before registering
  them" to use if instead of switch statement for error checking

Changes since [v3]

- Series reabse on top of [rfc]

- Fixed incorrect goto label in "thermal: qoriq: Pass data to
  qoriq_tmu_calibration()"
  
- Added REGS_TRITSR() register description to "thermal: qoriq: Do
  not report invalid temperature reading"
  
- Reworded commit message of "thermal: qoriq: Remove unnecessary
  DT node is NULL check"

Changes since [v2]

- Patches rebased on v5.1-rc1

Changes since [v1]

- Rebased on "linus" branch of
  git.kernel.org/pub/scm/linux/kernel/git/evalenti/linux-soc-thermal.git
  that included latest chagnes adding multi-sensors support

- Dropped

thermal: qoriq: Add support for multiple thremal sites
thermal: qoriq: Be more strict when parsing
thermal: qoriq: Simplify error handling in qoriq_tmu_get_sensor_id()

  since they are no longer relevant

- Added

thermal: qoriq: Don't store struct thermal_zone_device reference
thermal: qoriq: Add local struct qoriq_sensor pointer
thermal: qoriq: Embed per-sensor data into struct qoriq_tmu_data
thermal: qoriq: Pass data to qoriq_tmu_register_tmu_zone() directly

  to simplify latest codebase

- Changed "thermal: qoriq: Do not report invalid temperature
  reading" to use regmap_read_poll_timeout() to make sure that
  tmu_get_temp() waits for fist sample to be ready before
  reporting it. This case is triggered on my setup if
  qoriq_thermal is compiled as a module

[v1] lore.kernel.org/lkml/20190218191141.3729-1-andrew.smir...@gmail.com
[v2] lore.kernel.org/lkml/2019000508.26325-1-andrew.smir...@gmail.com
[v3] lore.kernel.org/lkml/20190401041418.5999-1-andrew.smir...@gmail.com
[v4] lore.kernel.org/lkml/20190413082748.29990-1-andrew.smir...@gmail.com
[v5] lore.kernel.org/lkml/20190424064830.18179-1-andrew.smir...@gmail.com
[v6] lore.kernel.org/lkml/20190821012612.7823-1-andrew.smir...@gmail.com
[rfc] lore.kernel.org/lkml/20190404080647.8173-1-daniel.lezc...@linaro.org

Andrey Smirnov (12):
  thermal: qoriq: Add local struct device pointer
  thermal: qoriq: Don't store struct thermal_zone_device reference
  thermal: qoriq: Add local struct qoriq_sensor pointer
  thermal: qoriq: Embed per-sensor data into struct qoriq_tmu_data
  thermal: qoriq: Pass data to qoriq_tmu_register_tmu_zone() directly
  thermal: qoriq: Pass data to qoriq_tmu_calibration() directly
  thermal: qoriq: Drop unnecessary drvdata cleanup
  thermal: qoriq: Convert driver to use regmap API
  thermal: qoriq: Enable all sensors before registering them
  thermal: qoriq: Do not report invalid temperature reading
  thermal_hwmon: Add devres wrapper for thermal_add_hwmon_sysfs()
  thermal: qoriq: Add hwmon support

 drivers/thermal/qoriq_thermal.c | 252 +---
 drivers/thermal/thermal_hwmon.c |  28 
 drivers/thermal/thermal_hwmon.h |   7 +
 3 files changed, 167 insertions(+), 120 deletions(-)

-- 
2.21.0

[PATCH v7 06/12] thermal: qoriq: Pass data to qoriq_tmu_calibration() directly

[Andrey Smirnov]

We can simplify error cleanup code if instead of passing a "struct
platform_device *" to qoriq_tmu_calibration() and deriving a bunch of
pointers from it, we pass those pointers directly. This way we won't
be force to call platform_set_drvdata() as early in qoriq_tmu_probe()
and need to have "platform_set_drvdata(pdev, NULL);" in error path.

Signed-off-by: Andrey Smirnov 
Reviewed-by: Daniel Lezcano 
Tested-by: Lucas Stach 
Cc: Chris Healy 
Cc: Lucas Stach 
Cc: Eduardo Valentin 
Cc: Daniel Lezcano 
Cc: Angus Ainslie (Purism) 
Cc: linux-...@nxp.com
Cc: linux...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/thermal/qoriq_thermal.c | 17 -
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c
index 5b9f2a31d275..af596c3342d0 100644
--- a/drivers/thermal/qoriq_thermal.c
+++ b/drivers/thermal/qoriq_thermal.c
@@ -144,16 +144,16 @@ static int qoriq_tmu_register_tmu_zone(struct device *dev,
return 0;
 }
 
-static int qoriq_tmu_calibration(struct platform_device *pdev)
+static int qoriq_tmu_calibration(struct device *dev,
+struct qoriq_tmu_data *data)
 {
int i, val, len;
u32 range[4];
const u32 *calibration;
-   struct device_node *np = pdev->dev.of_node;
-   struct qoriq_tmu_data *data = platform_get_drvdata(pdev);
+   struct device_node *np = dev->of_node;
 
if (of_property_read_u32_array(np, "fsl,tmu-range", range, 4)) {
-   dev_err(>dev, "missing calibration range.\n");
+   dev_err(dev, "missing calibration range.\n");
return -ENODEV;
}
 
@@ -165,7 +165,7 @@ static int qoriq_tmu_calibration(struct platform_device 
*pdev)
 
calibration = of_get_property(np, "fsl,tmu-calibration", );
if (calibration == NULL || len % 8) {
-   dev_err(>dev, "invalid calibration data.\n");
+   dev_err(dev, "invalid calibration data.\n");
return -ENODEV;
}
 
@@ -203,8 +203,6 @@ static int qoriq_tmu_probe(struct platform_device *pdev)
if (!data)
return -ENOMEM;
 
-   platform_set_drvdata(pdev, data);
-
data->little_endian = of_property_read_bool(np, "little-endian");
 
data->regs = devm_platform_ioremap_resource(pdev, 0);
@@ -225,7 +223,7 @@ static int qoriq_tmu_probe(struct platform_device *pdev)
 
qoriq_tmu_init_device(data);/* TMU initialization */
 
-   ret = qoriq_tmu_calibration(pdev);  /* TMU calibration */
+   ret = qoriq_tmu_calibration(dev, data); /* TMU calibration */
if (ret < 0)
goto err;
 
@@ -236,11 +234,12 @@ static int qoriq_tmu_probe(struct platform_device *pdev)
goto err;
}
 
+   platform_set_drvdata(pdev, data);
+
return 0;
 
 err:
clk_disable_unprepare(data->clk);
-   platform_set_drvdata(pdev, NULL);
 
return ret;
 }
-- 
2.21.0

[PATCH v7 04/12] thermal: qoriq: Embed per-sensor data into struct qoriq_tmu_data

[Andrey Smirnov]

Embed per-sensor data into struct qoriq_tmu_data so we can drop the
code allocating it. This also allows us to get rid of per-sensor back
reference to struct qoriq_tmu_data since now its address can be
calculated using container_of().

Signed-off-by: Andrey Smirnov 
Cc: Chris Healy 
Cc: Lucas Stach 
Cc: Zhang Rui 
Cc: Eduardo Valentin 
Cc: Daniel Lezcano 
Cc: Angus Ainslie (Purism) 
Cc: linux-...@nxp.com
Cc: linux...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/thermal/qoriq_thermal.c | 23 ---
 1 file changed, 8 insertions(+), 15 deletions(-)

diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c
index ae22836c471d..f8f5228d83af 100644
--- a/drivers/thermal/qoriq_thermal.c
+++ b/drivers/thermal/qoriq_thermal.c
@@ -60,13 +60,10 @@ struct qoriq_tmu_regs {
u32 ttr3cr; /* Temperature Range 3 Control Register */
 };
 
-struct qoriq_tmu_data;
-
 /*
  * Thermal zone data
  */
 struct qoriq_sensor {
-   struct qoriq_tmu_data   *qdata;
int id;
 };
 
@@ -74,9 +71,14 @@ struct qoriq_tmu_data {
struct qoriq_tmu_regs __iomem *regs;
struct clk *clk;
bool little_endian;
-   struct qoriq_sensor *sensor[SITES_MAX];
+   struct qoriq_sensor sensor[SITES_MAX];
 };
 
+static struct qoriq_tmu_data *qoriq_sensor_to_data(struct qoriq_sensor *s)
+{
+   return container_of(s, struct qoriq_tmu_data, sensor[s->id]);
+}
+
 static void tmu_write(struct qoriq_tmu_data *p, u32 val, void __iomem *addr)
 {
if (p->little_endian)
@@ -96,7 +98,7 @@ static u32 tmu_read(struct qoriq_tmu_data *p, void __iomem 
*addr)
 static int tmu_get_temp(void *p, int *temp)
 {
struct qoriq_sensor *qsensor = p;
-   struct qoriq_tmu_data *qdata = qsensor->qdata;
+   struct qoriq_tmu_data *qdata = qoriq_sensor_to_data(qsensor);
u32 val;
 
val = tmu_read(qdata, >regs->site[qsensor->id].tritsr);
@@ -116,19 +118,10 @@ static int qoriq_tmu_register_tmu_zone(struct 
platform_device *pdev)
 
for (id = 0; id < SITES_MAX; id++) {
struct thermal_zone_device *tzd;
-   struct qoriq_sensor *sensor;
+   struct qoriq_sensor *sensor = >sensor[id];
int ret;
 
-   sensor = devm_kzalloc(>dev,
- sizeof(struct qoriq_sensor),
- GFP_KERNEL);
-   if (!qdata->sensor[id])
-   return -ENOMEM;
-
-   qdata->sensor[id] = sensor;
-
sensor->id = id;
-   sensor->qdata = qdata;
 
tzd = devm_thermal_zone_of_sensor_register(>dev, id,
   sensor,
-- 
2.21.0

[PATCH v7 03/12] thermal: qoriq: Add local struct qoriq_sensor pointer

[Andrey Smirnov]

Add local struct qoriq_sensor pointer in qoriq_tmu_register_tmu_zone()
for brevity.

Signed-off-by: Andrey Smirnov 
Cc: Chris Healy 
Cc: Lucas Stach 
Cc: Zhang Rui 
Cc: Eduardo Valentin 
Cc: Daniel Lezcano 
Cc: Angus Ainslie (Purism) 
Cc: linux-...@nxp.com
Cc: linux...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/thermal/qoriq_thermal.c | 14 +-
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c
index b471c226f06b..ae22836c471d 100644
--- a/drivers/thermal/qoriq_thermal.c
+++ b/drivers/thermal/qoriq_thermal.c
@@ -116,18 +116,22 @@ static int qoriq_tmu_register_tmu_zone(struct 
platform_device *pdev)
 
for (id = 0; id < SITES_MAX; id++) {
struct thermal_zone_device *tzd;
+   struct qoriq_sensor *sensor;
int ret;
 
-   qdata->sensor[id] = devm_kzalloc(>dev,
-   sizeof(struct qoriq_sensor), GFP_KERNEL);
+   sensor = devm_kzalloc(>dev,
+ sizeof(struct qoriq_sensor),
+ GFP_KERNEL);
if (!qdata->sensor[id])
return -ENOMEM;
 
-   qdata->sensor[id]->id = id;
-   qdata->sensor[id]->qdata = qdata;
+   qdata->sensor[id] = sensor;
+
+   sensor->id = id;
+   sensor->qdata = qdata;
 
tzd = devm_thermal_zone_of_sensor_register(>dev, id,
-  qdata->sensor[id],
+  sensor,
   _tz_ops);
ret = PTR_ERR_OR_ZERO(tzd);
if (ret) {
-- 
2.21.0

[RESEND PATCH v2] Bluetooth: Retry configure request if result is L2CAP_CONF_UNKNOWN

[Andrey Smirnov]

Due to:

 * Current implementation of l2cap_config_rsp() dropping BT
   connection if sender of configuration response replied with unknown
   option failure (Result=0x0003/L2CAP_CONF_UNKNOWN)

 * Current implementation of l2cap_build_conf_req() adding
   L2CAP_CONF_RFC(0x04) option to initial configure request sent by
   the Linux host.

devices that do no recongninze L2CAP_CONF_RFC, such as Xbox One S
controllers, will get stuck in endless connect -> configure ->
disconnect loop, never connect and be generaly unusable.

To avoid this problem add code to do the following:

 1. Parse the body of response L2CAP_CONF_UNKNOWN and, in case of
unsupported option being RFC, clear L2CAP_FEAT_ERTM and
L2CAP_FEAT_STREAMING from connection's feature mask (in order to
prevent RFC option from being added going forward)

 2. Retry configuration step the same way it's done for
L2CAP_CONF_UNACCEPT

Signed-off-by: Andrey Smirnov 
Cc: Pierre-Loup A. Griffais 
Cc: Florian Dollinger 
Cc: Marcel Holtmann 
Cc: Johan Hedberg 
Cc: linux-blueto...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---

Changes since [v1]:

   - Patch simplified to simply clear L2CAP_FEAT_ERTM |
 L2CAP_FEAT_STREAMING from feat_mask when device flags RFC options
 as unknown

[v1] lore.kernel.org/r/20190208025828.30901-1-andrew.smir...@gmail.com

 net/bluetooth/l2cap_core.c | 58 ++
 1 file changed, 58 insertions(+)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index dfc1edb168b7..77b65870b064 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -4216,6 +4216,49 @@ static inline int l2cap_config_req(struct l2cap_conn 
*conn,
return err;
 }
 
+static inline int l2cap_config_rsp_unknown(struct l2cap_conn *conn,
+  struct l2cap_chan *chan,
+  const u8 *data,
+  int len)
+{
+   char req[64];
+
+   if (!len || len > sizeof(req) -  sizeof(struct l2cap_conf_req))
+   return -ECONNRESET;
+
+   while (len--) {
+   const u8 option_type = *data++;
+
+   BT_DBG("chan %p, unknown option type: %u", chan,  option_type);
+
+   /* "...Hints shall not be included in the Response and
+* shall not be the sole cause for rejecting the
+* Request.."
+*/
+   if (option_type & L2CAP_CONF_HINT)
+   return -ECONNRESET;
+
+   switch (option_type) {
+   case L2CAP_CONF_RFC:
+   /* Clearing the following feature should
+* prevent RFC option from being added next
+* connection attempt
+*/
+   conn->feat_mask &= ~(L2CAP_FEAT_ERTM |
+L2CAP_FEAT_STREAMING);
+   break;
+   default:
+   return -ECONNRESET;
+   }
+   }
+
+   len = l2cap_build_conf_req(chan, req, sizeof(req));
+   l2cap_send_cmd(conn, l2cap_get_ident(conn), L2CAP_CONF_REQ, len, req);
+   chan->num_conf_req++;
+
+   return 0;
+}
+
 static inline int l2cap_config_rsp(struct l2cap_conn *conn,
   struct l2cap_cmd_hdr *cmd, u16 cmd_len,
   u8 *data)
@@ -4271,6 +4314,21 @@ static inline int l2cap_config_rsp(struct l2cap_conn 
*conn,
}
goto done;
 
+   case L2CAP_CONF_UNKNOWN:
+   if (chan->num_conf_rsp <= L2CAP_CONF_MAX_CONF_RSP) {
+   if (l2cap_config_rsp_unknown(conn, chan, rsp->data,
+len) < 0) {
+   l2cap_send_disconn_req(chan, ECONNRESET);
+   goto done;
+   }
+   break;
+   }
+   /* Once, chan->num_conf_rsp goes above
+* L2CAP_CONF_MAX_CONF_RSP we want to go down all the
+* way to default label (just like L2CAP_CONF_UNACCEPT
+* below)
+*/
+   /* fall through */
case L2CAP_CONF_UNACCEPT:
if (chan->num_conf_rsp <= L2CAP_CONF_MAX_CONF_RSP) {
char req[64];
-- 
2.21.0

Re: [PATCH] drm/bridge: tc358767: Expose test mode functionality via debugfs

[Andrey Smirnov]

On Mon, Aug 26, 2019 at 10:46 PM Tomi Valkeinen  wrote:
>
> Hi,
>
> On 26/08/2019 21:25, Andrey Smirnov wrote:
> > Presently, the driver code artificially limits test pattern mode to a
> > single pattern with fixed color selection. It being a kernel module
> > parameter makes switching "test patter" <-> "proper output" modes
> > on-the-fly clunky and outright impossible if the driver is built into
> > the kernel.
> >
> > To improve the situation a bit, convert current test pattern code to
> > use debugfs instead by exposing "TestCtl" register. This way old
> > "tc_test_pattern=1" functionality can be emulated via:
> >
> >  echo -n 0x78146312 > tstctl
> >
> > and switch back to regular mode can be done with:
> >
> >  echo -n 0x78146310 > tstctl
>
> It might be worth explaining the format in the commit msg or in a
> comment in the driver.
>

Good point. Will do if this is the format going forward.

> > Note that switching to any of the test patterns, will NOT trigger link
> > re-establishment whereas switching to normal operation WILL. This is
> > done so:
> >
> > a) we can isolate and verify (e)DP link functionality by switching to
> > one of the test patters
> >
> > b) trigger a link re-establishment by switching back to normal mode
> >
> > Signed-off-by: Andrey Smirnov 
> > Cc: Andrzej Hajda 
> > Cc: Laurent Pinchart 
> > Cc: Tomi Valkeinen 
> > Cc: Cory Tusar 
> > Cc: Chris Healy 
> > Cc: Lucas Stach 
> > Cc: dri-de...@lists.freedesktop.org
> > Cc: linux-kernel@vger.kernel.org
> > ---
> >   drivers/gpu/drm/bridge/tc358767.c | 137 ++
> >   1 file changed, 101 insertions(+), 36 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/bridge/tc358767.c 
> > b/drivers/gpu/drm/bridge/tc358767.c
> > index 6308d93ad91d..7a795b613ed0 100644
> > --- a/drivers/gpu/drm/bridge/tc358767.c
> > +++ b/drivers/gpu/drm/bridge/tc358767.c
> > @@ -17,6 +17,7 @@
> >
> >   #include 
> >   #include 
> > +#include 
> >   #include 
> >   #include 
> >   #include 
> > @@ -222,11 +223,10 @@
> >   #define COLOR_B GENMASK(15, 8)
> >   #define ENI2CFILTER BIT(4)
> >   #define COLOR_BAR_MODE  GENMASK(1, 0)
> > +#define COLOR_BAR_MODE_NORMAL0
> >   #define COLOR_BAR_MODE_BARS 2
> > -#define PLL_DBG  0x0a04
> >
> > -static bool tc_test_pattern;
> > -module_param_named(test, tc_test_pattern, bool, 0644);
> > +#define PLL_DBG  0x0a04
> >
> >   struct tc_edp_link {
> >   struct drm_dp_link  base;
> > @@ -789,16 +789,6 @@ static int tc_set_video_mode(struct tc_data *tc,
> >   if (ret)
> >   return ret;
> >
> > - /* Test pattern settings */
> > - ret = regmap_write(tc->regmap, TSTCTL,
> > -FIELD_PREP(COLOR_R, 120) |
> > -FIELD_PREP(COLOR_G, 20) |
> > -FIELD_PREP(COLOR_B, 99) |
> > -ENI2CFILTER |
> > -FIELD_PREP(COLOR_BAR_MODE, COLOR_BAR_MODE_BARS));
> > - if (ret)
> > - return ret;
> > -
> >   /* DP Main Stream Attributes */
> >   vid_sync_dly = hsync_len + left_margin + mode->hdisplay;
> >   ret = regmap_write(tc->regmap, DP0_VIDSYNCDELAY,
> > @@ -1150,14 +1140,6 @@ static int tc_stream_enable(struct tc_data *tc)
> >
> >   dev_dbg(tc->dev, "enable video stream\n");
> >
> > - /* PXL PLL setup */
> > - if (tc_test_pattern) {
> > - ret = tc_pxl_pll_en(tc, clk_get_rate(tc->refclk),
> > - 1000 * tc->mode.clock);
> > - if (ret)
> > - return ret;
> > - }
> > -
> >   ret = tc_set_video_mode(tc, >mode);
> >   if (ret)
> >   return ret;
> > @@ -1186,12 +1168,8 @@ static int tc_stream_enable(struct tc_data *tc)
> >   if (ret)
> >   return ret;
> >   /* Set input interface */
> > - value = DP0_AUDSRC_NO_INPUT;
> > - if (tc_test_pattern)
> > - value |= DP0_VIDSRC_COLOR_BAR;
> > - else
> > - value |= DP0_VIDSRC_DPI_RX;
> > - ret = regmap_write(tc->regmap, SYSCTRL, value);
> > + ret = regmap_write(tc->regmap, SYSCTRL,
> > +   

Re: [PATCH] drm/bridge: tc358767: Expose test mode functionality via debugfs

[Andrey Smirnov]

On Tue, Aug 27, 2019 at 1:06 AM Laurent Pinchart
 wrote:
>
> Hi Andrey,
>
> On Mon, Aug 26, 2019 at 09:24:57PM -0700, Andrey Smirnov wrote:
> > On Mon, Aug 26, 2019 at 3:08 PM Laurent Pinchart wrote:
> > > On Mon, Aug 26, 2019 at 11:25:24AM -0700, Andrey Smirnov wrote:
> > > > Presently, the driver code artificially limits test pattern mode to a
> > > > single pattern with fixed color selection. It being a kernel module
> > > > parameter makes switching "test patter" <-> "proper output" modes
> > > > on-the-fly clunky and outright impossible if the driver is built into
> > > > the kernel.
> > > >
> > > > To improve the situation a bit, convert current test pattern code to
> > > > use debugfs instead by exposing "TestCtl" register. This way old
> > > > "tc_test_pattern=1" functionality can be emulated via:
> > > >
> > > > echo -n 0x78146312 > tstctl
> > > >
> > > > and switch back to regular mode can be done with:
> > > >
> > > > echo -n 0x78146310 > tstctl
> > >
> > > Can't we make this more userfriendly by exposing either a test pattern
> > > index, or a string ?
> >
> > We could, but then a) it would require more code in the driver b) the
> > files wouldn't correspond directly to something described in the
> > part's datasheet. Just didn't seem worth it to me.
>
> Could you then provide me with the datasheet ? :-)

Is this a rhetoric question or are you seriously asking? If its the
latter I can ping you off the list.

> The whole point of a
> driver is to avoid needing detailed knowledge of the device's internals
> in userspace.
>

You won't avoid needing detailed knowledge of the device's internals
if you don't have a priori knowledge in the form of a agreed upon/well
known abstraction you are exposing from the driver. There is no such
abstraction in this case. Whether you present "tstctl" that takes a
magic value or "red", "green", "blue" and "pattern" taking numbers and
special strings, as a user, you still would have to go read the driver
code in order to figure out how that stuff works.

Given how this is an obscure _debug_ feature for a niche part, I think
exposing raw register and leaving a comment in the driver source code
explaining how it works is reasonably user-friendly (for all 10 - 15
unique users that this feature would ever have).

To avoid any further back and forth of this subject, how about the
following. If this is up to me, then I'd like to move forward to v2
with the interface as is. If you feel strongly about this and insist
on your vision of the interface, please let me know what it looks like
(e.g. is what I described above good enough) and I'll rework v2 to
have that.

> > > Do all bits in the register need to be controlled
> > > from userspace ?
> >
> > Pretty much, yes. It's formatted as RR_GG_BB_X_M, where R, G, B
> > specifies color used for various patterns, X is irrelevant and M
> > specifies test pattern to use.
> >
> > > > Note that switching to any of the test patterns, will NOT trigger link
> > > > re-establishment whereas switching to normal operation WILL. This is
> > > > done so:
> > > >
> > > > a) we can isolate and verify (e)DP link functionality by switching to
> > > >one of the test patters
> > > >
> > > > b) trigger a link re-establishment by switching back to normal mode
> > > >
> > > > Signed-off-by: Andrey Smirnov 
> > > > Cc: Andrzej Hajda 
> > > > Cc: Laurent Pinchart 
> > > > Cc: Tomi Valkeinen 
> > > > Cc: Cory Tusar 
> > > > Cc: Chris Healy 
> > > > Cc: Lucas Stach 
> > > > Cc: dri-de...@lists.freedesktop.org
> > > > Cc: linux-kernel@vger.kernel.org
> > > > ---
> > > >  drivers/gpu/drm/bridge/tc358767.c | 137 ++
> > > >  1 file changed, 101 insertions(+), 36 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/bridge/tc358767.c 
> > > > b/drivers/gpu/drm/bridge/tc358767.c
> > > > index 6308d93ad91d..7a795b613ed0 100644
> > > > --- a/drivers/gpu/drm/bridge/tc358767.c
> > > > +++ b/drivers/gpu/drm/bridge/tc358767.c
> > > > @@ -17,6 +17,7 @@
> > > >
> > > >  #include 
> > > >  #include 
> > > > +#include 
> > > >  #include 
> > > >  #include 
> > > >  #include 
> > &g

Re: [PATCH] ARM: imx: Drop imx_anatop_init()

[Andrey Smirnov]

On Sat, Aug 24, 2019 at 11:31 AM Shawn Guo  wrote:
>
> On Thu, Aug 22, 2019 at 05:33:13PM +, Leonard Crestez wrote:
> > On 31.07.2019 21:01, Andrey Smirnov wrote:
> > > With commit b5bbe2235361 ("usb: phy: mxs: Disable external charger
> > > detect in mxs_phy_hw_init()") in tree all of the necessary charger
> > > setup is done by the USB PHY driver which covers all of the affected
> > > i.MX6 SoCs.
> > >
> > > NOTE: Imx_anatop_init() was also called for i.MX7D, but looking at its
> > > datasheet it appears to have a different USB PHY IP block, so
> > > executing i.MX6 charger disable configuration seems unnecessary.
> > >
> > > -void __init imx_anatop_init(void)
> > > -{
> > > -   anatop = syscon_regmap_lookup_by_compatible("fsl,imx6q-anatop");
> > > -   if (IS_ERR(anatop)) {
> > > -   pr_err("%s: failed to find imx6q-anatop regmap!\n", __func__);
> > > -   return;
> > > -   }
> >
> > This patch breaks suspend on imx6 in linux-next because the "anatop"
> > regmap is no longer initialized. This was found via bisect but
> > no_console_suspend prints a helpful stack anyway:
> >
> > (regmap_read) from [] (imx_anatop_enable_weak2p5+0x28/0x70)
> > (imx_anatop_enable_weak2p5) from []
> > (imx_anatop_pre_suspend+0x18/0x64)
> > (imx_anatop_pre_suspend) from [] (imx6q_pm_enter+0x60/0x16c)
> > (imx6q_pm_enter) from [] (suspend_devices_and_enter+0x7d4/0xcbc)
> > (suspend_devices_and_enter) from [] (pm_suspend+0x7b8/0x904)
> > (pm_suspend) from [] (state_store+0x68/0xc8)
>
> I dropped it from my branch for now.  Thanks for reporting!
>

OK, it sounds like I can submit a v2 that only removes
imx_anatop_usb_chrg_detect_disable() and keeps imx_anatop_init()
intact.

Thanks,
Andrey Smirnov

Re: [PATCH] ARM: dts: vf610-zii-scu4-aib: Drop "rs485-rts-delay" property

[Andrey Smirnov]

On Sat, Aug 24, 2019 at 12:09 PM Shawn Guo  wrote:
>
> On Mon, Aug 19, 2019 at 08:13:01PM -0700, Andrey Smirnov wrote:
> > LPUART driver does not support specifying "rs485-rts-delay"
> > property. Drop it.
>
> If so, we need to fix bindings/serial/fsl-lpuart.txt in the meantime?
>

Yeah, good point. Will submit a separate patch for that.

Thanks,
Andrey Smirnov

Re: [PATCH] ARM: dts: vf610-zii-dev-rev-b: Drop redundant I2C properties

[Andrey Smirnov]

On Sat, Aug 24, 2019 at 12:12 PM Shawn Guo  wrote:
>
> On Mon, Aug 19, 2019 at 08:19:52PM -0700, Andrey Smirnov wrote:
> > Drop redundant I2C properties that are already specified in
> > vf610-zii-dev.dtsi
> >
> > Signed-off-by: Andrey Smirnov 
> > Cc: Shawn Guo 
> > Cc: Chris Healy 
> > Cc: Fabio Estevam 
> > Cc: linux-arm-ker...@lists.infradead.org
> > Cc: linux-kernel@vger.kernel.org
> > ---
> >  arch/arm/boot/dts/vf610-zii-dev-rev-b.dts | 10 --
> >  1 file changed, 10 deletions(-)
> >
> > diff --git a/arch/arm/boot/dts/vf610-zii-dev-rev-b.dts 
> > b/arch/arm/boot/dts/vf610-zii-dev-rev-b.dts
> > index 48086c5e8549..e500911ce0a5 100644
> > --- a/arch/arm/boot/dts/vf610-zii-dev-rev-b.dts
> > +++ b/arch/arm/boot/dts/vf610-zii-dev-rev-b.dts
> > @@ -323,11 +323,6 @@
> >  };
> >
> >   {
> > - clock-frequency = <10>;
> > - pinctrl-names = "default";
> > - pinctrl-0 = <_i2c0>;
>
> pinctrl for i2c0 is not same as what vf610-zii-dev.dtsi has.

The only difference I can see is in pinctrl-names so I am assuming
that's what you mean. Not configuring I2C recovery on Rev B board was
not intentional. I'll update commit log in v2.

Thanks,
Andrey Smirnov

[PATCH] ARM: dts: vf610-zii-scu4-aib: Use generic names for DT nodes

[Andrey Smirnov]

The devicetree specification recommends using generic node names.

Some ZII dts files already follow such recommendation, but some don't,
so use generic node names for consistency among the ZII dts files.

Signed-off-by: Andrey Smirnov 
Cc: Shawn Guo 
Cc: Chris Healy 
Cc: Cory Tusar 
Cc: Fabio Estevam 
Cc: linux-arm-ker...@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
---
 arch/arm/boot/dts/vf610-zii-scu4-aib.dts | 24 
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/arch/arm/boot/dts/vf610-zii-scu4-aib.dts 
b/arch/arm/boot/dts/vf610-zii-scu4-aib.dts
index 45a978defbdc..6edd682dbd29 100644
--- a/arch/arm/boot/dts/vf610-zii-scu4-aib.dts
+++ b/arch/arm/boot/dts/vf610-zii-scu4-aib.dts
@@ -417,7 +417,7 @@
pinctrl-0 = <_dspi1>;
status = "okay";
 
-   spi-flash@0 {
+   flash@0 {
#address-cells = <1>;
#size-cells = <1>;
compatible = "jedec,spi-nor";
@@ -430,7 +430,7 @@
};
};
 
-   spi-flash@1 {
+   flash@1 {
#address-cells = <1>;
#size-cells = <1>;
compatible = "jedec,spi-nor";
@@ -519,7 +519,7 @@
#gpio-cells = <2>;
};
 
-   lm75@48 {
+   temp-sensor@48 {
compatible = "national,lm75";
reg = <0x48>;
};
@@ -534,7 +534,7 @@
reg = <0x52>;
};
 
-   ds1682@6b {
+   elapsed-time-recorder@6b {
compatible = "dallas,ds1682";
reg = <0x6b>;
};
@@ -551,7 +551,7 @@
reg = <0x38>;
};
 
-   adt7411@4a {
+   adc@4a {
compatible = "adi,adt7411";
reg = <0x4a>;
};
@@ -563,7 +563,7 @@
pinctrl-0 = <_i2c2>;
status = "okay";
 
-   gpio9: sx1503q@20 {
+   gpio9: io-expander@20 {
compatible = "semtech,sx1503q";
pinctrl-names = "default";
pinctrl-0 = <_sx1503_20>;
@@ -574,12 +574,12 @@
interrupts = <31 IRQ_TYPE_EDGE_FALLING>;
};
 
-   lm75@4e {
+   temp-sensor@4e {
compatible = "national,lm75";
reg = <0x4e>;
};
 
-   lm75@4f {
+   temp-sensor@4f {
compatible = "national,lm75";
reg = <0x4f>;
};
@@ -591,17 +591,17 @@
reg = <0x23>;
};
 
-   adt7411@4a {
+   adc@4a {
compatible = "adi,adt7411";
reg = <0x4a>;
};
 
-   at24c08@54 {
+   eeprom@54 {
compatible = "atmel,24c08";
reg = <0x54>;
};
 
-   tca9548@70 {
+   i2c-mux@70 {
compatible = "nxp,pca9548";
pinctrl-names = "default";
#address-cells = <1>;
@@ -639,7 +639,7 @@
};
};
 
-   tca9548@71 {
+   i2c-mux@71 {
compatible = "nxp,pca9548";
pinctrl-names = "default";
reg = <0x71>;
-- 
2.21.0

[PATCH] ARM: dts: vf610-zii-scu4-aib: Configure IRQ line for GPIO expander

[Andrey Smirnov]

Configure IRQ line for SX1503 GPIO expander. We already have
appropriate pinmux entry and all that is missing is "interrupt-parent"
and "interrupts" properties. Add them.

Signed-off-by: Andrey Smirnov 
Cc: Shawn Guo 
Cc: Chris Healy 
Cc: Cory Tusar 
Cc: Fabio Estevam 
Cc: linux-arm-ker...@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
---
 arch/arm/boot/dts/vf610-zii-scu4-aib.dts | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/boot/dts/vf610-zii-scu4-aib.dts 
b/arch/arm/boot/dts/vf610-zii-scu4-aib.dts
index e6c3621079e0..45a978defbdc 100644
--- a/arch/arm/boot/dts/vf610-zii-scu4-aib.dts
+++ b/arch/arm/boot/dts/vf610-zii-scu4-aib.dts
@@ -570,6 +570,8 @@
#gpio-cells = <2>;
reg = <0x20>;
gpio-controller;
+   interrupt-parent = <>;
+   interrupts = <31 IRQ_TYPE_EDGE_FALLING>;
};
 
lm75@4e {
-- 
2.21.0

Re: [PATCH] ARM: imx: Drop imx_anatop_init()

[Andrey Smirnov]

On Thu, Aug 22, 2019 at 10:33 AM Leonard Crestez
 wrote:
>
> On 31.07.2019 21:01, Andrey Smirnov wrote:
> > With commit b5bbe2235361 ("usb: phy: mxs: Disable external charger
> > detect in mxs_phy_hw_init()") in tree all of the necessary charger
> > setup is done by the USB PHY driver which covers all of the affected
> > i.MX6 SoCs.
> >
> > NOTE: Imx_anatop_init() was also called for i.MX7D, but looking at its
> > datasheet it appears to have a different USB PHY IP block, so
> > executing i.MX6 charger disable configuration seems unnecessary.
> >
> > -void __init imx_anatop_init(void)
> > -{
> > - anatop = syscon_regmap_lookup_by_compatible("fsl,imx6q-anatop");
> > - if (IS_ERR(anatop)) {
> > - pr_err("%s: failed to find imx6q-anatop regmap!\n", __func__);
> > - return;
> > - }
>
> This patch breaks suspend on imx6 in linux-next because the "anatop"
> regmap is no longer initialized. This was found via bisect but
> no_console_suspend prints a helpful stack anyway:
>
> (regmap_read) from [] (imx_anatop_enable_weak2p5+0x28/0x70)
> (imx_anatop_enable_weak2p5) from []
> (imx_anatop_pre_suspend+0x18/0x64)
> (imx_anatop_pre_suspend) from [] (imx6q_pm_enter+0x60/0x16c)
> (imx6q_pm_enter) from [] (suspend_devices_and_enter+0x7d4/0xcbc)
> (suspend_devices_and_enter) from [] (pm_suspend+0x7b8/0x904)
> (pm_suspend) from [] (state_store+0x68/0xc8)
>

My bad, completely missed that fact that anatop was a global variable
in  imx_anatop_init(). Sorry about that.

> Minimal fix looks like this:
>
> --- arch/arm/mach-imx/anatop.c
> +++ arch/arm/mach-imx/anatop.c
> @@ -111,6 +111,12 @@ void __init imx_init_revision_from_anatop(void)
>   digprog = readl_relaxed(anatop_base + offset);
>   iounmap(anatop_base);
>
> +   anatop = syscon_regmap_lookup_by_compatible("fsl,imx6q-anatop");
> +   if (IS_ERR(anatop)) {
> +   pr_err("failed to find imx6q-anatop regmap!\n");
> +   return;
> +   }
>
> Since all SOCs that called imx_anatop_init also call
> imx_init_revision_from_anatop this might be an acceptable solution,
> unless there is some limitation preventing early regmap lookup.
>

Would making every function that uses anatop explicitly request it via
syscon_regmap_lookup_by_compatible("fsl,imx6q-anatop") be too much of
a code duplication? This way we won't need to worry if
imx_init_revision_from_anatop() was called before any of them are
used.

Thanks,
Andrey Smirnov

[PATCH v2] ARM: dts: vf610-zii-cfu1: Slow I2C0 down to 100 kHz

[Andrey Smirnov]

Fiber-optic modules attached to the bus are only rated to work at
100 kHz, so decrease the bus frequency to accommodate that.

Signed-off-by: Andrey Smirnov 
Cc: Shawn Guo 
Cc: Chris Healy 
Cc: Fabio Estevam 
Cc: linux-arm-ker...@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
---

Changes since [v1]:

 - Spelling fixes

[v1] lore.kernel.org/lkml/20190820030804.8892-1-andrew.smir...@gmail.com

 arch/arm/boot/dts/vf610-zii-cfu1.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/vf610-zii-cfu1.dts 
b/arch/arm/boot/dts/vf610-zii-cfu1.dts
index ff460a1de85a..28732249cfc0 100644
--- a/arch/arm/boot/dts/vf610-zii-cfu1.dts
+++ b/arch/arm/boot/dts/vf610-zii-cfu1.dts
@@ -207,7 +207,7 @@
 };
 
  {
-   clock-frequency = <40>;
+   clock-frequency = <10>;
pinctrl-names = "default";
pinctrl-0 = <_i2c0>;
status = "okay";
-- 
2.21.0

Re: [PATCH] ARM: dts: vf610-zii-cfu1: Slow I2C0 down to 100kHz

[Andrey Smirnov]

On Tue, Aug 20, 2019 at 8:29 AM Andrew Lunn  wrote:
>
> On Mon, Aug 19, 2019 at 08:08:04PM -0700, Andrey Smirnov wrote:
> > Fiber-optic module attached to the bus is only rated to work at
> > 100kHz, so drop the bus frequncy to accomodate that.
>
> Hi Andrey
>
> Did you review all the other ZII platforms? I could imaging the same
> problem happening else where.
>

Yes, AFAICT, fiber-optic modules are present only on SCU4
(vf610-zii-scu4-aib.dts), CFU1 (vf610-zii-cfu1.dts) and VF610 Dev
board rev. B/C (vf610-zii-dev*.dts[i]). Of all three only CFU1 has
corresponding I2C bus running @ 400 kHz.

Thanks,
Andrey Smirnov

Re: [PATCH] ARM: dts: vf610-zii-cfu1: Slow I2C0 down to 100kHz

[Andrey Smirnov]

On Tue, Aug 20, 2019 at 7:41 AM Marc Gonzalez  wrote:
>
> On 20/08/2019 05:08, Andrey Smirnov wrote:
>
> > Fiber-optic module attached to the bus is only rated to work at
> > 100kHz, so drop the bus frequncy to accomodate that.
>
> s/100kHz/100 kHz
> s/frequncy/frequency
> s/accomodate/accommodate
>

Will fix in v2.

Thanks,
Andrey Smirnov

[RESEND PATCH v6 08/12] thermal: qoriq: Convert driver to use regmap API

[Andrey Smirnov]

Convert driver to use regmap API, drop custom LE/BE IO helpers and
simplify bit manipulation using regmap_update_bits(). This also allows
us to convert some register initialization to use loops and adds
convenient debug access to TMU registers via debugfs.

Signed-off-by: Andrey Smirnov 
Reviewed-by: Daniel Lezcano 
Tested-by: Lucas Stach 
Cc: Chris Healy 
Cc: Lucas Stach 
Cc: Eduardo Valentin 
Cc: Daniel Lezcano 
Cc: Angus Ainslie (Purism) 
Cc: linux-...@nxp.com
Cc: linux...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/thermal/qoriq_thermal.c | 159 +++-
 1 file changed, 74 insertions(+), 85 deletions(-)

diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c
index 80fe9adcc313..08167ebe6060 100644
--- a/drivers/thermal/qoriq_thermal.c
+++ b/drivers/thermal/qoriq_thermal.c
@@ -8,6 +8,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include "thermal_core.h"
@@ -17,48 +18,27 @@
 /*
  * QorIQ TMU Registers
  */
-struct qoriq_tmu_site_regs {
-   u32 tritsr; /* Immediate Temperature Site Register */
-   u32 tratsr; /* Average Temperature Site Register */
-   u8 res0[0x8];
-};
 
-struct qoriq_tmu_regs {
-   u32 tmr;/* Mode Register */
+#define REGS_TMR   0x000   /* Mode Register */
 #define TMR_DISABLE0x0
 #define TMR_ME 0x8000
 #define TMR_ALPF   0x0c00
-   u32 tsr;/* Status Register */
-   u32 tmtmir; /* Temperature measurement interval Register */
+
+#define REGS_TMTMIR0x008   /* Temperature measurement interval Register */
 #define TMTMIR_DEFAULT 0x000f
-   u8 res0[0x14];
-   u32 tier;   /* Interrupt Enable Register */
+
+#define REGS_TIER  0x020   /* Interrupt Enable Register */
 #define TIER_DISABLE   0x0
-   u32 tidr;   /* Interrupt Detect Register */
-   u32 tiscr;  /* Interrupt Site Capture Register */
-   u32 ticscr; /* Interrupt Critical Site Capture Register */
-   u8 res1[0x10];
-   u32 tmhtcrh;/* High Temperature Capture Register */
-   u32 tmhtcrl;/* Low Temperature Capture Register */
-   u8 res2[0x8];
-   u32 tmhtitr;/* High Temperature Immediate Threshold */
-   u32 tmhtatr;/* High Temperature Average Threshold */
-   u32 tmhtactr;   /* High Temperature Average Crit Threshold */
-   u8 res3[0x24];
-   u32 ttcfgr; /* Temperature Configuration Register */
-   u32 tscfgr; /* Sensor Configuration Register */
-   u8 res4[0x78];
-   struct qoriq_tmu_site_regs site[SITES_MAX];
-   u8 res5[0x9f8];
-   u32 ipbrr0; /* IP Block Revision Register 0 */
-   u32 ipbrr1; /* IP Block Revision Register 1 */
-   u8 res6[0x310];
-   u32 ttr0cr; /* Temperature Range 0 Control Register */
-   u32 ttr1cr; /* Temperature Range 1 Control Register */
-   u32 ttr2cr; /* Temperature Range 2 Control Register */
-   u32 ttr3cr; /* Temperature Range 3 Control Register */
-};
 
+#define REGS_TTCFGR0x080   /* Temperature Configuration Register */
+#define REGS_TSCFGR0x084   /* Sensor Configuration Register */
+
+#define REGS_TRITSR(n) (0x100 + 16 * (n)) /* Immediate Temperature
+   * Site Register
+   */
+#define REGS_TTRnCR(n) (0xf10 + 4 * (n)) /* Temperature Range n
+  * Control Register
+  */
 /*
  * Thermal zone data
  */
@@ -67,8 +47,7 @@ struct qoriq_sensor {
 };
 
 struct qoriq_tmu_data {
-   struct qoriq_tmu_regs __iomem *regs;
-   bool little_endian;
+   struct regmap *regmap;
struct qoriq_sensor sensor[SITES_MAX];
 };
 
@@ -77,29 +56,13 @@ static struct qoriq_tmu_data *qoriq_sensor_to_data(struct 
qoriq_sensor *s)
return container_of(s, struct qoriq_tmu_data, sensor[s->id]);
 }
 
-static void tmu_write(struct qoriq_tmu_data *p, u32 val, void __iomem *addr)
-{
-   if (p->little_endian)
-   iowrite32(val, addr);
-   else
-   iowrite32be(val, addr);
-}
-
-static u32 tmu_read(struct qoriq_tmu_data *p, void __iomem *addr)
-{
-   if (p->little_endian)
-   return ioread32(addr);
-   else
-   return ioread32be(addr);
-}
-
 static int tmu_get_temp(void *p, int *temp)
 {
struct qoriq_sensor *qsensor = p;
struct qoriq_tmu_data *qdata = qoriq_sensor_to_data(qsensor);
u32 val;
 
-   val = tmu_read(qdata, >regs->site[qsensor->id].tritsr);
+   regmap_read(qdata->regmap, REGS_TRITSR(qsensor->id), );
*temp = (val & 0xff) * 1000;
 
return 0;
@@ -135,7 +98,8 @@ static int qoriq_tm

[RESEND PATCH v6 05/12] thermal: qoriq: Pass data to qoriq_tmu_register_tmu_zone() directly

[Andrey Smirnov]

Pass all necessary data to qoriq_tmu_register_tmu_zone() directly
instead of passing a paltform device and then deriving it. This is
done as a first step to simplify resource deallocation code.

Signed-off-by: Andrey Smirnov 
Acked-by: Daniel Lezcano 
Tested-by: Lucas Stach 
Cc: Chris Healy 
Cc: Lucas Stach 
Cc: Eduardo Valentin 
Cc: Daniel Lezcano 
Cc: Angus Ainslie (Purism) 
Cc: linux-...@nxp.com
Cc: linux...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/thermal/qoriq_thermal.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c
index 4d5c74173f08..61700881d9f0 100644
--- a/drivers/thermal/qoriq_thermal.c
+++ b/drivers/thermal/qoriq_thermal.c
@@ -109,9 +109,9 @@ static const struct thermal_zone_of_device_ops tmu_tz_ops = 
{
.get_temp = tmu_get_temp,
 };
 
-static int qoriq_tmu_register_tmu_zone(struct platform_device *pdev)
+static int qoriq_tmu_register_tmu_zone(struct device *dev,
+  struct qoriq_tmu_data *qdata)
 {
-   struct qoriq_tmu_data *qdata = platform_get_drvdata(pdev);
int id, sites = 0;
 
for (id = 0; id < SITES_MAX; id++) {
@@ -120,7 +120,7 @@ static int qoriq_tmu_register_tmu_zone(struct 
platform_device *pdev)
 
sensor->id = id;
 
-   tzd = devm_thermal_zone_of_sensor_register(>dev, id,
+   tzd = devm_thermal_zone_of_sensor_register(dev, id,
   sensor,
   _tz_ops);
if (IS_ERR(tzd)) {
@@ -216,7 +216,7 @@ static int qoriq_tmu_probe(struct platform_device *pdev)
if (ret < 0)
goto err_tmu;
 
-   ret = qoriq_tmu_register_tmu_zone(pdev);
+   ret = qoriq_tmu_register_tmu_zone(dev, data);
if (ret < 0) {
dev_err(dev, "Failed to register sensors\n");
ret = -ENODEV;
-- 
2.21.0

[RESEND PATCH v6 02/12] thermal: qoriq: Don't store struct thermal_zone_device reference

[Andrey Smirnov]

Struct thermal_zone_device reference stored as sensor's private data
isn't really used anywhere in the code. Drop it.

Signed-off-by: Andrey Smirnov 
Acked-by: Daniel Lezcano 
Tested-by: Lucas Stach 
Cc: Chris Healy 
Cc: Lucas Stach 
Cc: Eduardo Valentin 
Cc: Daniel Lezcano 
Cc: Angus Ainslie (Purism) 
Cc: linux-...@nxp.com
Cc: linux...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/thermal/qoriq_thermal.c | 15 +--
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c
index 91f9f49d2776..6d40b9788266 100644
--- a/drivers/thermal/qoriq_thermal.c
+++ b/drivers/thermal/qoriq_thermal.c
@@ -65,7 +65,6 @@ struct qoriq_tmu_data;
  * Thermal zone data
  */
 struct qoriq_sensor {
-   struct thermal_zone_device  *tzd;
struct qoriq_tmu_data   *qdata;
int id;
 };
@@ -114,6 +113,8 @@ static int qoriq_tmu_register_tmu_zone(struct 
platform_device *pdev)
int id, sites = 0;
 
for (id = 0; id < SITES_MAX; id++) {
+   struct thermal_zone_device *tzd;
+
qdata->sensor[id] = devm_kzalloc(>dev,
sizeof(struct qoriq_sensor), GFP_KERNEL);
if (!qdata->sensor[id])
@@ -121,13 +122,15 @@ static int qoriq_tmu_register_tmu_zone(struct 
platform_device *pdev)
 
qdata->sensor[id]->id = id;
qdata->sensor[id]->qdata = qdata;
-   qdata->sensor[id]->tzd = devm_thermal_zone_of_sensor_register(
-   >dev, id, qdata->sensor[id], _tz_ops);
-   if (IS_ERR(qdata->sensor[id]->tzd)) {
-   if (PTR_ERR(qdata->sensor[id]->tzd) == -ENODEV)
+
+   tzd = devm_thermal_zone_of_sensor_register(>dev, id,
+  qdata->sensor[id],
+  _tz_ops);
+   if (IS_ERR(tzd)) {
+   if (PTR_ERR(tzd) == -ENODEV)
continue;
else
-   return PTR_ERR(qdata->sensor[id]->tzd);
+   return PTR_ERR(tzd);
}
 
sites |= 0x1 << (15 - id);
-- 
2.21.0

[RESEND PATCH v6 01/12] thermal: qoriq: Add local struct device pointer

[Andrey Smirnov]

Use a local "struct device *dev" for brevity. No functional change
intended.

Signed-off-by: Andrey Smirnov 
Acked-by: Daniel Lezcano 
Tested-by: Lucas Stach 
Cc: Chris Healy 
Cc: Lucas Stach 
Cc: Eduardo Valentin 
Cc: Daniel Lezcano 
Cc: Angus Ainslie (Purism) 
Cc: linux-...@nxp.com
Cc: linux...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/thermal/qoriq_thermal.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c
index 7b364933bfb1..91f9f49d2776 100644
--- a/drivers/thermal/qoriq_thermal.c
+++ b/drivers/thermal/qoriq_thermal.c
@@ -192,8 +192,9 @@ static int qoriq_tmu_probe(struct platform_device *pdev)
int ret;
struct qoriq_tmu_data *data;
struct device_node *np = pdev->dev.of_node;
+   struct device *dev = >dev;
 
-   data = devm_kzalloc(>dev, sizeof(struct qoriq_tmu_data),
+   data = devm_kzalloc(dev, sizeof(struct qoriq_tmu_data),
GFP_KERNEL);
if (!data)
return -ENOMEM;
@@ -204,7 +205,7 @@ static int qoriq_tmu_probe(struct platform_device *pdev)
 
data->regs = of_iomap(np, 0);
if (!data->regs) {
-   dev_err(>dev, "Failed to get memory region\n");
+   dev_err(dev, "Failed to get memory region\n");
ret = -ENODEV;
goto err_iomap;
}
@@ -217,7 +218,7 @@ static int qoriq_tmu_probe(struct platform_device *pdev)
 
ret = qoriq_tmu_register_tmu_zone(pdev);
if (ret < 0) {
-   dev_err(>dev, "Failed to register sensors\n");
+   dev_err(dev, "Failed to register sensors\n");
ret = -ENODEV;
goto err_iomap;
}
-- 
2.21.0

[RESEND PATCH v6 03/12] thermal: qoriq: Add local struct qoriq_sensor pointer

[Andrey Smirnov]

Add local struct qoriq_sensor pointer in qoriq_tmu_register_tmu_zone()
for brevity.

Signed-off-by: Andrey Smirnov 
Cc: Chris Healy 
Cc: Lucas Stach 
Cc: Zhang Rui 
Cc: Eduardo Valentin 
Cc: Daniel Lezcano 
Cc: Angus Ainslie (Purism) 
Cc: linux-...@nxp.com
Cc: linux...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/thermal/qoriq_thermal.c | 14 +-
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c
index 6d40b9788266..d74c6c494f77 100644
--- a/drivers/thermal/qoriq_thermal.c
+++ b/drivers/thermal/qoriq_thermal.c
@@ -114,17 +114,21 @@ static int qoriq_tmu_register_tmu_zone(struct 
platform_device *pdev)
 
for (id = 0; id < SITES_MAX; id++) {
struct thermal_zone_device *tzd;
+   struct qoriq_sensor *sensor;
 
-   qdata->sensor[id] = devm_kzalloc(>dev,
-   sizeof(struct qoriq_sensor), GFP_KERNEL);
+   sensor = devm_kzalloc(>dev,
+ sizeof(struct qoriq_sensor),
+ GFP_KERNEL);
if (!qdata->sensor[id])
return -ENOMEM;
 
-   qdata->sensor[id]->id = id;
-   qdata->sensor[id]->qdata = qdata;
+   qdata->sensor[id] = sensor;
+
+   sensor->id = id;
+   sensor->qdata = qdata;
 
tzd = devm_thermal_zone_of_sensor_register(>dev, id,
-  qdata->sensor[id],
+  sensor,
   _tz_ops);
if (IS_ERR(tzd)) {
if (PTR_ERR(tzd) == -ENODEV)
-- 
2.21.0

[RESEND PATCH v6 11/12] thermal_hwmon: Add devres wrapper for thermal_add_hwmon_sysfs()

[Andrey Smirnov]

Add devres wrapper for thermal_add_hwmon_sysfs() to simplify driver
code.

Signed-off-by: Andrey Smirnov 
Reviewed-by: Daniel Lezcano 
Tested-by: Lucas Stach 
Cc: Chris Healy 
Cc: Lucas Stach 
Cc: Eduardo Valentin 
Cc: Daniel Lezcano 
Cc: Angus Ainslie (Purism) 
Cc: linux-...@nxp.com
Cc: linux...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/thermal/thermal_hwmon.c | 28 
 drivers/thermal/thermal_hwmon.h |  7 +++
 2 files changed, 35 insertions(+)

diff --git a/drivers/thermal/thermal_hwmon.c b/drivers/thermal/thermal_hwmon.c
index 40c69a533b24..4e79524182e1 100644
--- a/drivers/thermal/thermal_hwmon.c
+++ b/drivers/thermal/thermal_hwmon.c
@@ -244,3 +244,31 @@ void thermal_remove_hwmon_sysfs(struct thermal_zone_device 
*tz)
kfree(hwmon);
 }
 EXPORT_SYMBOL_GPL(thermal_remove_hwmon_sysfs);
+
+static void devm_thermal_hwmon_release(struct device *dev, void *res)
+{
+   thermal_remove_hwmon_sysfs(*(struct thermal_zone_device **)res);
+}
+
+int devm_thermal_add_hwmon_sysfs(struct thermal_zone_device *tz)
+{
+   struct thermal_zone_device **ptr;
+   int ret;
+
+   ptr = devres_alloc(devm_thermal_hwmon_release, sizeof(*ptr),
+  GFP_KERNEL);
+   if (!ptr)
+   return -ENOMEM;
+
+   ret = thermal_add_hwmon_sysfs(tz);
+   if (ret) {
+   devres_free(ptr);
+   return ret;
+   }
+
+   *ptr = tz;
+   devres_add(>device, ptr);
+
+   return ret;
+}
+EXPORT_SYMBOL_GPL(devm_thermal_add_hwmon_sysfs);
diff --git a/drivers/thermal/thermal_hwmon.h b/drivers/thermal/thermal_hwmon.h
index a160b9d62dd0..1a9d65f6a6a8 100644
--- a/drivers/thermal/thermal_hwmon.h
+++ b/drivers/thermal/thermal_hwmon.h
@@ -17,6 +17,7 @@
 
 #ifdef CONFIG_THERMAL_HWMON
 int thermal_add_hwmon_sysfs(struct thermal_zone_device *tz);
+int devm_thermal_add_hwmon_sysfs(struct thermal_zone_device *tz);
 void thermal_remove_hwmon_sysfs(struct thermal_zone_device *tz);
 #else
 static inline int
@@ -25,6 +26,12 @@ thermal_add_hwmon_sysfs(struct thermal_zone_device *tz)
return 0;
 }
 
+static inline int
+devm_thermal_add_hwmon_sysfs(struct thermal_zone_device *tz)
+{
+   return 0;
+}
+
 static inline void
 thermal_remove_hwmon_sysfs(struct thermal_zone_device *tz)
 {
-- 
2.21.0

[RESEND PATCH v6 00/12] QorIQ TMU multi-sensor and HWMON support

[Andrey Smirnov]

Everyone:

This series contains patches adding support for HWMON integration, bug
fixes and general improvements (hopefully) for TMU driver I made while
working on it on i.MX8MQ.

Feedback is welcome!

Thanks,
Andrey Smirnov

Changes since [v5]

- Rebased on recent linux-next, dropped "thermal: qoriq: Remove
  unnecessary DT node is NULL check" since it is already in the
  tree

- Dropped dependency on [rfc]

Changes since [v4]

- Collected Tested-by from Lucas

- Collected Reviewed-by from Daniel

- Converted "thermal: qoriq: Enable all sensors before registering
  them" to use if instead of switch statement for error checking

Changes since [v3]

- Series reabse on top of [rfc]

- Fixed incorrect goto label in "thermal: qoriq: Pass data to
  qoriq_tmu_calibration()"
  
- Added REGS_TRITSR() register description to "thermal: qoriq: Do
  not report invalid temperature reading"
  
- Reworded commit message of "thermal: qoriq: Remove unnecessary
  DT node is NULL check"

Changes since [v2]

- Patches rebased on v5.1-rc1

Changes since [v1]

- Rebased on "linus" branch of
  git.kernel.org/pub/scm/linux/kernel/git/evalenti/linux-soc-thermal.git
  that included latest chagnes adding multi-sensors support

- Dropped

thermal: qoriq: Add support for multiple thremal sites
thermal: qoriq: Be more strict when parsing
thermal: qoriq: Simplify error handling in qoriq_tmu_get_sensor_id()

  since they are no longer relevant

- Added

thermal: qoriq: Don't store struct thermal_zone_device reference
thermal: qoriq: Add local struct qoriq_sensor pointer
thermal: qoriq: Embed per-sensor data into struct qoriq_tmu_data
thermal: qoriq: Pass data to qoriq_tmu_register_tmu_zone() directly

  to simplify latest codebase

- Changed "thermal: qoriq: Do not report invalid temperature
  reading" to use regmap_read_poll_timeout() to make sure that
  tmu_get_temp() waits for fist sample to be ready before
  reporting it. This case is triggered on my setup if
  qoriq_thermal is compiled as a module

[v1] lore.kernel.org/lkml/20190218191141.3729-1-andrew.smir...@gmail.com
[v2] lore.kernel.org/lkml/2019000508.26325-1-andrew.smir...@gmail.com
[v3] lore.kernel.org/lkml/20190401041418.5999-1-andrew.smir...@gmail.com
[v4] lore.kernel.org/lkml/20190413082748.29990-1-andrew.smir...@gmail.com
[v5] lore.kernel.org/lkml/20190424064830.18179-1-andrew.smir...@gmail.com
[rfc] lore.kernel.org/lkml/20190404080647.8173-1-daniel.lezc...@linaro.org


Andrey Smirnov (12):
  thermal: qoriq: Add local struct device pointer
  thermal: qoriq: Don't store struct thermal_zone_device reference
  thermal: qoriq: Add local struct qoriq_sensor pointer
  thermal: qoriq: Embed per-sensor data into struct qoriq_tmu_data
  thermal: qoriq: Pass data to qoriq_tmu_register_tmu_zone() directly
  thermal: qoriq: Pass data to qoriq_tmu_calibration() directly
  thermal: qoriq: Convert driver to use devm_ioremap()
  thermal: qoriq: Convert driver to use regmap API
  thermal: qoriq: Enable all sensors before registering them
  thermal: qoriq: Do not report invalid temperature reading
  thermal_hwmon: Add devres wrapper for thermal_add_hwmon_sysfs()
  thermal: qoriq: Add hwmon support

 drivers/thermal/qoriq_thermal.c | 272 
 drivers/thermal/thermal_hwmon.c |  28 
 drivers/thermal/thermal_hwmon.h |   7 +
 3 files changed, 175 insertions(+), 132 deletions(-)

-- 
2.21.0

[RESEND PATCH v6 12/12] thermal: qoriq: Add hwmon support

[Andrey Smirnov]

Expose thermal readings as a HWMON device, so that it could be
accessed using lm-sensors.

Signed-off-by: Andrey Smirnov 
Reviewed-by: Daniel Lezcano 
Tested-by: Lucas Stach 
Cc: Chris Healy 
Cc: Lucas Stach 
Cc: Eduardo Valentin 
Cc: Daniel Lezcano 
Cc: Angus Ainslie (Purism) 
Cc: linux-...@nxp.com
Cc: linux...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/thermal/qoriq_thermal.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c
index 2f8c5feb1a25..56f9927cf63c 100644
--- a/drivers/thermal/qoriq_thermal.c
+++ b/drivers/thermal/qoriq_thermal.c
@@ -12,6 +12,7 @@
 #include 
 
 #include "thermal_core.h"
+#include "thermal_hwmon.h"
 
 #define SITES_MAX  16
 
@@ -115,6 +116,11 @@ static int qoriq_tmu_register_tmu_zone(struct device *dev,
regmap_write(qdata->regmap, REGS_TMR, TMR_DISABLE);
return ret;
}
+
+   if (devm_thermal_add_hwmon_sysfs(tzd))
+   dev_warn(dev,
+"Failed to add hwmon sysfs attributes\n");
+
}
 
return 0;
-- 
2.21.0

[RESEND PATCH v6 10/12] thermal: qoriq: Do not report invalid temperature reading

[Andrey Smirnov]

Before returning measured temperature data to upper layer we need to
make sure that the reading was marked as "valid" to avoid reporting
bogus data.

Signed-off-by: Andrey Smirnov 
Reviewed-by: Daniel Lezcano 
Tested-by: Lucas Stach 
Cc: Chris Healy 
Cc: Lucas Stach 
Cc: Eduardo Valentin 
Cc: Daniel Lezcano 
Cc: Angus Ainslie (Purism) 
Cc: linux-...@nxp.com
Cc: linux...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/thermal/qoriq_thermal.c | 19 ++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c
index 5f964f806187..2f8c5feb1a25 100644
--- a/drivers/thermal/qoriq_thermal.c
+++ b/drivers/thermal/qoriq_thermal.c
@@ -37,6 +37,7 @@
 #define REGS_TRITSR(n) (0x100 + 16 * (n)) /* Immediate Temperature
* Site Register
*/
+#define TRITSR_V   BIT(31)
 #define REGS_TTRnCR(n) (0xf10 + 4 * (n)) /* Temperature Range n
   * Control Register
   */
@@ -62,8 +63,24 @@ static int tmu_get_temp(void *p, int *temp)
struct qoriq_sensor *qsensor = p;
struct qoriq_tmu_data *qdata = qoriq_sensor_to_data(qsensor);
u32 val;
+   /*
+* REGS_TRITSR(id) has the following layout:
+*
+* 31  ... 7 6 5 4 3 2 1 0
+*  V  TEMP
+*
+* Where V bit signifies if the measurement is ready and is
+* within sensor range. TEMP is an 8 bit value representing
+* temperature in C.
+*/
+   if (regmap_read_poll_timeout(qdata->regmap,
+REGS_TRITSR(qsensor->id),
+val,
+val & TRITSR_V,
+USEC_PER_MSEC,
+10 * USEC_PER_MSEC))
+   return -ENODATA;
 
-   regmap_read(qdata->regmap, REGS_TRITSR(qsensor->id), );
*temp = (val & 0xff) * 1000;
 
return 0;
-- 
2.21.0

[RESEND PATCH v6 04/12] thermal: qoriq: Embed per-sensor data into struct qoriq_tmu_data

[Andrey Smirnov]

Embed per-sensor data into struct qoriq_tmu_data so we can drop the
code allocating it. This also allows us to get rid of per-sensor back
reference to struct qoriq_tmu_data since now its address can be
caluclated using container_of().

Signed-off-by: Andrey Smirnov 
Cc: Chris Healy 
Cc: Lucas Stach 
Cc: Zhang Rui 
Cc: Eduardo Valentin 
Cc: Daniel Lezcano 
Cc: Angus Ainslie (Purism) 
Cc: linux-...@nxp.com
Cc: linux...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/thermal/qoriq_thermal.c | 23 ---
 1 file changed, 8 insertions(+), 15 deletions(-)

diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c
index d74c6c494f77..4d5c74173f08 100644
--- a/drivers/thermal/qoriq_thermal.c
+++ b/drivers/thermal/qoriq_thermal.c
@@ -59,22 +59,24 @@ struct qoriq_tmu_regs {
u32 ttr3cr; /* Temperature Range 3 Control Register */
 };
 
-struct qoriq_tmu_data;
-
 /*
  * Thermal zone data
  */
 struct qoriq_sensor {
-   struct qoriq_tmu_data   *qdata;
int id;
 };
 
 struct qoriq_tmu_data {
struct qoriq_tmu_regs __iomem *regs;
bool little_endian;
-   struct qoriq_sensor *sensor[SITES_MAX];
+   struct qoriq_sensor sensor[SITES_MAX];
 };
 
+static struct qoriq_tmu_data *qoriq_sensor_to_data(struct qoriq_sensor *s)
+{
+   return container_of(s, struct qoriq_tmu_data, sensor[s->id]);
+}
+
 static void tmu_write(struct qoriq_tmu_data *p, u32 val, void __iomem *addr)
 {
if (p->little_endian)
@@ -94,7 +96,7 @@ static u32 tmu_read(struct qoriq_tmu_data *p, void __iomem 
*addr)
 static int tmu_get_temp(void *p, int *temp)
 {
struct qoriq_sensor *qsensor = p;
-   struct qoriq_tmu_data *qdata = qsensor->qdata;
+   struct qoriq_tmu_data *qdata = qoriq_sensor_to_data(qsensor);
u32 val;
 
val = tmu_read(qdata, >regs->site[qsensor->id].tritsr);
@@ -114,18 +116,9 @@ static int qoriq_tmu_register_tmu_zone(struct 
platform_device *pdev)
 
for (id = 0; id < SITES_MAX; id++) {
struct thermal_zone_device *tzd;
-   struct qoriq_sensor *sensor;
-
-   sensor = devm_kzalloc(>dev,
- sizeof(struct qoriq_sensor),
- GFP_KERNEL);
-   if (!qdata->sensor[id])
-   return -ENOMEM;
-
-   qdata->sensor[id] = sensor;
+   struct qoriq_sensor *sensor = >sensor[id];
 
sensor->id = id;
-   sensor->qdata = qdata;
 
tzd = devm_thermal_zone_of_sensor_register(>dev, id,
   sensor,
-- 
2.21.0

[RESEND PATCH v6 07/12] thermal: qoriq: Convert driver to use devm_ioremap()

[Andrey Smirnov]

Convert driver to use devm_ioremap() to simplify memory deallocation
and error handling code. No functional change intended.

Signed-off-by: Andrey Smirnov 
Reviewed-by: Daniel Lezcano 
Tested-by: Lucas Stach 
Cc: Chris Healy 
Cc: Lucas Stach 
Cc: Eduardo Valentin 
Cc: Daniel Lezcano 
Cc: Angus Ainslie (Purism) 
Cc: linux-...@nxp.com
Cc: linux...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/thermal/qoriq_thermal.c | 20 ++--
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c
index 62d7a0efb837..80fe9adcc313 100644
--- a/drivers/thermal/qoriq_thermal.c
+++ b/drivers/thermal/qoriq_thermal.c
@@ -193,6 +193,7 @@ static int qoriq_tmu_probe(struct platform_device *pdev)
struct qoriq_tmu_data *data;
struct device_node *np = pdev->dev.of_node;
struct device *dev = >dev;
+   struct resource *io;
 
data = devm_kzalloc(dev, sizeof(struct qoriq_tmu_data),
GFP_KERNEL);
@@ -201,7 +202,13 @@ static int qoriq_tmu_probe(struct platform_device *pdev)
 
data->little_endian = of_property_read_bool(np, "little-endian");
 
-   data->regs = of_iomap(np, 0);
+   io = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+   if (!io) {
+   dev_err(dev, "Failed to get memory region\n");
+   return -ENODEV;
+   }
+
+   data->regs = devm_ioremap(dev, io->start, resource_size(io));
if (!data->regs) {
dev_err(dev, "Failed to get memory region\n");
return -ENODEV;
@@ -211,23 +218,17 @@ static int qoriq_tmu_probe(struct platform_device *pdev)
 
ret = qoriq_tmu_calibration(dev, data); /* TMU calibration */
if (ret < 0)
-   goto err_tmu;
+   return ret;
 
ret = qoriq_tmu_register_tmu_zone(dev, data);
if (ret < 0) {
dev_err(dev, "Failed to register sensors\n");
-   ret = -ENODEV;
-   goto err_tmu;
+   return -ENODEV;
}
 
platform_set_drvdata(pdev, data);
 
return 0;
-
-err_tmu:
-   iounmap(data->regs);
-
-   return ret;
 }
 
 static int qoriq_tmu_remove(struct platform_device *pdev)
@@ -237,7 +238,6 @@ static int qoriq_tmu_remove(struct platform_device *pdev)
/* Disable monitoring */
tmu_write(data, TMR_DISABLE, >regs->tmr);
 
-   iounmap(data->regs);
platform_set_drvdata(pdev, NULL);
 
return 0;
-- 
2.21.0

[RESEND PATCH v6 09/12] thermal: qoriq: Enable all sensors before registering them

[Andrey Smirnov]

Tmu_get_temp will get called as a part of sensor registration via
devm_thermal_zone_of_sensor_register(). To prevent it from retruning
bogus data we need to enable sensor monitoring before that. Looking at
the datasheet (i.MX8MQ RM) there doesn't seem to be any harm in
enabling them all, so, for the sake of simplicity, change the code to
do just that.

Signed-off-by: Andrey Smirnov 
Tested-by: Lucas Stach 
Cc: Chris Healy 
Cc: Lucas Stach 
Cc: Eduardo Valentin 
Cc: Daniel Lezcano 
Cc: Angus Ainslie (Purism) 
Cc: linux-...@nxp.com
Cc: linux...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/thermal/qoriq_thermal.c | 23 +++
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c
index 08167ebe6060..5f964f806187 100644
--- a/drivers/thermal/qoriq_thermal.c
+++ b/drivers/thermal/qoriq_thermal.c
@@ -23,6 +23,7 @@
 #define TMR_DISABLE0x0
 #define TMR_ME 0x8000
 #define TMR_ALPF   0x0c00
+#define TMR_MSITE_ALL  GENMASK(15, 0)
 
 #define REGS_TMTMIR0x008   /* Temperature measurement interval Register */
 #define TMTMIR_DEFAULT 0x000f
@@ -75,7 +76,10 @@ static const struct thermal_zone_of_device_ops tmu_tz_ops = {
 static int qoriq_tmu_register_tmu_zone(struct device *dev,
   struct qoriq_tmu_data *qdata)
 {
-   int id, sites = 0;
+   int id, ret;
+
+   regmap_write(qdata->regmap, REGS_TMR,
+TMR_MSITE_ALL | TMR_ME | TMR_ALPF);
 
for (id = 0; id < SITES_MAX; id++) {
struct thermal_zone_device *tzd;
@@ -86,21 +90,16 @@ static int qoriq_tmu_register_tmu_zone(struct device *dev,
tzd = devm_thermal_zone_of_sensor_register(dev, id,
   sensor,
   _tz_ops);
-   if (IS_ERR(tzd)) {
-   if (PTR_ERR(tzd) == -ENODEV)
+   ret = PTR_ERR_OR_ZERO(tzd);
+   if (ret) {
+   if (ret == -ENODEV)
continue;
-   else
-   return PTR_ERR(tzd);
-   }
 
-   sites |= 0x1 << (15 - id);
+   regmap_write(qdata->regmap, REGS_TMR, TMR_DISABLE);
+   return ret;
+   }
}
 
-   /* Enable monitoring */
-   if (sites != 0)
-   regmap_write(qdata->regmap, REGS_TMR,
-sites | TMR_ME | TMR_ALPF);
-
return 0;
 }
 
-- 
2.21.0

[RESEND PATCH v6 06/12] thermal: qoriq: Pass data to qoriq_tmu_calibration() directly

[Andrey Smirnov]

We can simplify error cleanup code if instead of passing a "struct
platform_device *" to qoriq_tmu_calibration() and deriving a bunch of
pointers from it, we pass those pointers directly. This way we won't
be force to call platform_set_drvdata() as early in qoriq_tmu_probe()
and consequently would be able to drop the "err_iomap" error path.

Signed-off-by: Andrey Smirnov 
Reviewed-by: Daniel Lezcano 
Tested-by: Lucas Stach 
Cc: Chris Healy 
Cc: Lucas Stach 
Cc: Eduardo Valentin 
Cc: Daniel Lezcano 
Cc: Angus Ainslie (Purism) 
Cc: linux-...@nxp.com
Cc: linux...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/thermal/qoriq_thermal.c | 24 ++--
 1 file changed, 10 insertions(+), 14 deletions(-)

diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c
index 61700881d9f0..62d7a0efb837 100644
--- a/drivers/thermal/qoriq_thermal.c
+++ b/drivers/thermal/qoriq_thermal.c
@@ -140,16 +140,16 @@ static int qoriq_tmu_register_tmu_zone(struct device *dev,
return 0;
 }
 
-static int qoriq_tmu_calibration(struct platform_device *pdev)
+static int qoriq_tmu_calibration(struct device *dev,
+struct qoriq_tmu_data *data)
 {
int i, val, len;
u32 range[4];
const u32 *calibration;
-   struct device_node *np = pdev->dev.of_node;
-   struct qoriq_tmu_data *data = platform_get_drvdata(pdev);
+   struct device_node *np = dev->of_node;
 
if (of_property_read_u32_array(np, "fsl,tmu-range", range, 4)) {
-   dev_err(>dev, "missing calibration range.\n");
+   dev_err(dev, "missing calibration range.\n");
return -ENODEV;
}
 
@@ -161,7 +161,7 @@ static int qoriq_tmu_calibration(struct platform_device 
*pdev)
 
calibration = of_get_property(np, "fsl,tmu-calibration", );
if (calibration == NULL || len % 8) {
-   dev_err(>dev, "invalid calibration data.\n");
+   dev_err(dev, "invalid calibration data.\n");
return -ENODEV;
}
 
@@ -199,20 +199,17 @@ static int qoriq_tmu_probe(struct platform_device *pdev)
if (!data)
return -ENOMEM;
 
-   platform_set_drvdata(pdev, data);
-
data->little_endian = of_property_read_bool(np, "little-endian");
 
data->regs = of_iomap(np, 0);
if (!data->regs) {
dev_err(dev, "Failed to get memory region\n");
-   ret = -ENODEV;
-   goto err_iomap;
+   return -ENODEV;
}
 
qoriq_tmu_init_device(data);/* TMU initialization */
 
-   ret = qoriq_tmu_calibration(pdev);  /* TMU calibration */
+   ret = qoriq_tmu_calibration(dev, data); /* TMU calibration */
if (ret < 0)
goto err_tmu;
 
@@ -220,17 +217,16 @@ static int qoriq_tmu_probe(struct platform_device *pdev)
if (ret < 0) {
dev_err(dev, "Failed to register sensors\n");
ret = -ENODEV;
-   goto err_iomap;
+   goto err_tmu;
}
 
+   platform_set_drvdata(pdev, data);
+
return 0;
 
 err_tmu:
iounmap(data->regs);
 
-err_iomap:
-   platform_set_drvdata(pdev, NULL);
-
return ret;
 }
 
-- 
2.21.0

[PATCH v8 08/16] crypto: caam - share definition for MAX_SDLEN

[Andrey Smirnov]

Both qi.h and cammalg_qi2.h seem to define identical versions of
MAX_SDLEN. Move it to desc_constr.h to avoid duplication.

Signed-off-by: Andrey Smirnov 
Cc: Chris Spencer 
Cc: Cory Tusar 
Cc: Chris Healy 
Cc: Lucas Stach 
Cc: Horia Geantă 
Cc: Aymen Sghaier 
Cc: Leonard Crestez 
Cc: linux-cry...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/crypto/caam/caamalg_qi2.h | 27 ---
 drivers/crypto/caam/desc_constr.h | 27 +++
 drivers/crypto/caam/qi.h  | 26 --
 3 files changed, 27 insertions(+), 53 deletions(-)

diff --git a/drivers/crypto/caam/caamalg_qi2.h 
b/drivers/crypto/caam/caamalg_qi2.h
index b450e2a25c1f..706736776b47 100644
--- a/drivers/crypto/caam/caamalg_qi2.h
+++ b/drivers/crypto/caam/caamalg_qi2.h
@@ -92,33 +92,6 @@ struct dpaa2_caam_priv_per_cpu {
struct dpaa2_io *dpio;
 };
 
-/*
- * The CAAM QI hardware constructs a job descriptor which points
- * to shared descriptor (as pointed by context_a of FQ to CAAM).
- * When the job descriptor is executed by deco, the whole job
- * descriptor together with shared descriptor gets loaded in
- * deco buffer which is 64 words long (each 32-bit).
- *
- * The job descriptor constructed by QI hardware has layout:
- *
- * HEADER  (1 word)
- * Shdesc ptr  (1 or 2 words)
- * SEQ_OUT_PTR (1 word)
- * Out ptr (1 or 2 words)
- * Out length  (1 word)
- * SEQ_IN_PTR  (1 word)
- * In ptr  (1 or 2 words)
- * In length   (1 word)
- *
- * The shdesc ptr is used to fetch shared descriptor contents
- * into deco buffer.
- *
- * Apart from shdesc contents, the total number of words that
- * get loaded in deco buffer are '8' or '11'. The remaining words
- * in deco buffer can be used for storing shared descriptor.
- */
-#define MAX_SDLEN  ((CAAM_DESC_BYTES_MAX - DESC_JOB_IO_LEN) / CAAM_CMD_SZ)
-
 /* Length of a single buffer in the QI driver memory cache */
 #define CAAM_QI_MEMCACHE_SIZE  512
 
diff --git a/drivers/crypto/caam/desc_constr.h 
b/drivers/crypto/caam/desc_constr.h
index 536f360bf131..1fe50a4fefaa 100644
--- a/drivers/crypto/caam/desc_constr.h
+++ b/drivers/crypto/caam/desc_constr.h
@@ -18,6 +18,33 @@
 #define CAAM_DESC_BYTES_MAX (CAAM_CMD_SZ * MAX_CAAM_DESCSIZE)
 #define DESC_JOB_IO_LEN (CAAM_CMD_SZ * 5 + CAAM_PTR_SZ * 3)
 
+/*
+ * The CAAM QI hardware constructs a job descriptor which points
+ * to shared descriptor (as pointed by context_a of FQ to CAAM).
+ * When the job descriptor is executed by deco, the whole job
+ * descriptor together with shared descriptor gets loaded in
+ * deco buffer which is 64 words long (each 32-bit).
+ *
+ * The job descriptor constructed by QI hardware has layout:
+ *
+ * HEADER  (1 word)
+ * Shdesc ptr  (1 or 2 words)
+ * SEQ_OUT_PTR (1 word)
+ * Out ptr (1 or 2 words)
+ * Out length  (1 word)
+ * SEQ_IN_PTR  (1 word)
+ * In ptr  (1 or 2 words)
+ * In length   (1 word)
+ *
+ * The shdesc ptr is used to fetch shared descriptor contents
+ * into deco buffer.
+ *
+ * Apart from shdesc contents, the total number of words that
+ * get loaded in deco buffer are '8' or '11'. The remaining words
+ * in deco buffer can be used for storing shared descriptor.
+ */
+#define MAX_SDLEN  ((CAAM_DESC_BYTES_MAX - DESC_JOB_IO_LEN) / CAAM_CMD_SZ)
+
 #ifdef DEBUG
 #define PRINT_POS do { printk(KERN_DEBUG "%02d: %s\n", desc_len(desc),\
  &__func__[sizeof("append")]); } while (0)
diff --git a/drivers/crypto/caam/qi.h b/drivers/crypto/caam/qi.h
index f93c9c7ed430..db0549549e3b 100644
--- a/drivers/crypto/caam/qi.h
+++ b/drivers/crypto/caam/qi.h
@@ -14,32 +14,6 @@
 #include "desc.h"
 #include "desc_constr.h"
 
-/*
- * CAAM hardware constructs a job descriptor which points to a shared 
descriptor
- * (as pointed by context_a of to-CAAM FQ).
- * When the job descriptor is executed by DECO, the whole job descriptor
- * together with shared descriptor gets loaded in DECO buffer, which is
- * 64 words (each 32-bit) long.
- *
- * The job descriptor constructed by CAAM hardware has the following layout:
- *
- * HEADER  (1 word)
- * Shdesc ptr  (1 or 2 words)
- * SEQ_OUT_PTR (1 word)
- * Out ptr (1 or 2 words)
- * Out length  (1 word)
- * SEQ_IN_PTR  (1 word)
- * In ptr  (1 or 2 words)
- * In length   (1 word)
- *
- * The shdesc ptr is used to fetch shared descriptor contents into DECO buffer.
- *
- * Apart from shdesc contents, the total number of words that get loaded in 
DECO
- * buffer are '8' or '11'. The remaining words in DECO buffer can be used for
- * storing shared descriptor.
- */
-#define MAX_SDLEN  ((CAAM_DESC_BYTES_MAX - DESC_JOB_IO_LEN) / CAAM_CMD_SZ)
-
 /* Length of a single buffer in the QI driver memory cache */
 #define CAAM_QI_MEMCACHE_SIZE  768
 
-- 
2.21.0

[PATCH v8 06/16] crypto: caam - use ioread64*_hi_lo in rd_reg64

[Andrey Smirnov]

Following the same transformation logic as outlined in previous commit
converting wr_reg64, convert rd_reg64 to use helpers from
 first. No functional change intended.

Signed-off-by: Andrey Smirnov 
Reviewed-by: Horia Geantă 
Cc: Chris Spencer 
Cc: Cory Tusar 
Cc: Chris Healy 
Cc: Lucas Stach 
Cc: Horia Geantă 
Cc: Aymen Sghaier 
Cc: Leonard Crestez 
Cc: linux-cry...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/crypto/caam/regs.h | 18 +-
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/crypto/caam/regs.h b/drivers/crypto/caam/regs.h
index 6acfef30a90c..4efc10534873 100644
--- a/drivers/crypto/caam/regs.h
+++ b/drivers/crypto/caam/regs.h
@@ -172,12 +172,20 @@ static inline void wr_reg64(void __iomem *reg, u64 data)
 
 static inline u64 rd_reg64(void __iomem *reg)
 {
-   if (!caam_imx && caam_little_end)
-   return ((u64)rd_reg32((u32 __iomem *)(reg) + 1) << 32 |
-   (u64)rd_reg32((u32 __iomem *)(reg)));
+   if (caam_little_end) {
+   if (caam_imx) {
+   u32 low, high;
 
-   return ((u64)rd_reg32((u32 __iomem *)(reg)) << 32 |
-   (u64)rd_reg32((u32 __iomem *)(reg) + 1));
+   high = ioread32(reg);
+   low  = ioread32(reg + sizeof(u32));
+
+   return low + ((u64)high << 32);
+   } else {
+   return ioread64(reg);
+   }
+   } else {
+   return ioread64be(reg);
+   }
 }
 #endif /* CONFIG_64BIT  */
 
-- 
2.21.0

[PATCH v8 05/16] crytpo: caam - make use of iowrite64*_hi_lo in wr_reg64

[Andrey Smirnov]

In order to be able to unify 64 and 32 bit implementations of
wr_reg64, let's convert it to use helpers from
 first. Here are the steps of the
transformation:

1. Inline wr_reg32 helpers:

if (!caam_imx && caam_little_end) {
if (caam_little_end) {
iowrite32(data >> 32, (u32 __iomem *)(reg) + 1);
iowrite32(data, (u32 __iomem *)(reg));
} else {
iowrite32be(data >> 32, (u32 __iomem *)(reg) + 1);
iowrite32be(data, (u32 __iomem *)(reg));
}
} else {
if (caam_little_end) {
iowrite32(data >> 32, (u32 __iomem *)(reg));
iowrite32(data, (u32 __iomem *)(reg) + 1);
} else {
iowrite32be(data >> 32, (u32 __iomem *)(reg));
iowrite32be(data, (u32 __iomem *)(reg) + 1);
}
}

2. Transfrom the conditionals such that the check for
'caam_little_end' is at the top level:

if (caam_little_end) {
if (!caam_imx) {
iowrite32(data >> 32, (u32 __iomem *)(reg) + 1);
iowrite32(data, (u32 __iomem *)(reg));
} else {
iowrite32(data >> 32, (u32 __iomem *)(reg));
iowrite32(data, (u32 __iomem *)(reg) + 1);
}
} else {
iowrite32be(data >> 32, (u32 __iomem *)(reg));
iowrite32be(data, (u32 __iomem *)(reg) + 1);
}

3. Invert the check for !caam_imx:

if (caam_little_end) {
if (caam_imx) {
iowrite32(data >> 32, (u32 __iomem *)(reg));
iowrite32(data, (u32 __iomem *)(reg) + 1);
} else {
iowrite32(data >> 32, (u32 __iomem *)(reg) + 1);
iowrite32(data, (u32 __iomem *)(reg));
}
} else {
iowrite32be(data >> 32, (u32 __iomem *)(reg));
iowrite32be(data, (u32 __iomem *)(reg) + 1);
}

4. Make use of iowrite64* helpers from 

if (caam_little_end) {
if (caam_imx) {
iowrite32(data >> 32, (u32 __iomem *)(reg));
iowrite32(data, (u32 __iomem *)(reg) + 1);
} else {
iowrite64(data, reg);
}
} else {
iowrite64be(data, reg);
}

No functional change intended.

Signed-off-by: Andrey Smirnov 
Reviewed-by: Horia Geantă 
Cc: Chris Spencer 
Cc: Cory Tusar 
Cc: Chris Healy 
Cc: Lucas Stach 
Cc: Horia Geantă 
Cc: Aymen Sghaier 
Cc: Leonard Crestez 
Cc: linux-cry...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/crypto/caam/regs.h | 14 +-
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/crypto/caam/regs.h b/drivers/crypto/caam/regs.h
index 7c7ea8af6a48..6acfef30a90c 100644
--- a/drivers/crypto/caam/regs.h
+++ b/drivers/crypto/caam/regs.h
@@ -12,6 +12,7 @@
 #include 
 #include 
 #include 
+#include 
 
 /*
  * Architecture-specific register access methods
@@ -157,12 +158,15 @@ static inline u64 rd_reg64(void __iomem *reg)
 #else /* CONFIG_64BIT */
 static inline void wr_reg64(void __iomem *reg, u64 data)
 {
-   if (!caam_imx && caam_little_end) {
-   wr_reg32((u32 __iomem *)(reg) + 1, data >> 32);
-   wr_reg32((u32 __iomem *)(reg), data);
+   if (caam_little_end) {
+   if (caam_imx) {
+   iowrite32(data >> 32, (u32 __iomem *)(reg));
+   iowrite32(data, (u32 __iomem *)(reg) + 1);
+   } else {
+   iowrite64(data, reg);
+   }
} else {
-   wr_reg32((u32 __iomem *)(reg), data >> 32);
-   wr_reg32((u32 __iomem *)(reg) + 1, data);
+   iowrite64be(data, reg);
}
 }
 
-- 
2.21.0

[PATCH v8 09/16] crypto: caam - make CAAM_PTR_SZ dynamic

[Andrey Smirnov]

In order to be able to configure CAAM pointer size at run-time, which
needed to support i.MX8MQ, which is 64-bit SoC with 32-bit pointer
size, convert CAAM_PTR_SZ to refer to a global variable of the same
name ("caam_ptr_sz") and adjust the rest of the code accordingly. No
functional change intended.

Signed-off-by: Andrey Smirnov 
Cc: Chris Spencer 
Cc: Cory Tusar 
Cc: Chris Healy 
Cc: Lucas Stach 
Cc: Horia Geantă 
Cc: Aymen Sghaier 
Cc: Leonard Crestez 
Cc: linux-cry...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/crypto/caam/caamalg.c |  2 +-
 drivers/crypto/caam/caamhash.c|  2 +-
 drivers/crypto/caam/caamrng.c |  2 +-
 drivers/crypto/caam/ctrl.c|  1 +
 drivers/crypto/caam/desc_constr.h | 12 +---
 drivers/crypto/caam/error.c   |  3 +++
 6 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/drivers/crypto/caam/caamalg.c b/drivers/crypto/caam/caamalg.c
index 947ba8ef487a..2053b3a71b48 100644
--- a/drivers/crypto/caam/caamalg.c
+++ b/drivers/crypto/caam/caamalg.c
@@ -74,7 +74,7 @@
 
 #define CHACHAPOLY_DESC_JOB_IO_LEN (AEAD_DESC_JOB_IO_LEN + CAAM_CMD_SZ * 6)
 
-#define DESC_MAX_USED_BYTES(CAAM_DESC_BYTES_MAX - DESC_JOB_IO_LEN)
+#define DESC_MAX_USED_BYTES(CAAM_DESC_BYTES_MAX - 
DESC_JOB_IO_LEN_MIN)
 #define DESC_MAX_USED_LEN  (DESC_MAX_USED_BYTES / CAAM_CMD_SZ)
 
 struct caam_alg_entry {
diff --git a/drivers/crypto/caam/caamhash.c b/drivers/crypto/caam/caamhash.c
index 8a07edb45dad..65399cb2a770 100644
--- a/drivers/crypto/caam/caamhash.c
+++ b/drivers/crypto/caam/caamhash.c
@@ -560,7 +560,7 @@ struct ahash_edesc {
dma_addr_t sec4_sg_dma;
int src_nents;
int sec4_sg_bytes;
-   u32 hw_desc[DESC_JOB_IO_LEN / sizeof(u32)] cacheline_aligned;
+   u32 hw_desc[DESC_JOB_IO_LEN_MAX / sizeof(u32)] cacheline_aligned;
struct sec4_sg_entry sec4_sg[0];
 };
 
diff --git a/drivers/crypto/caam/caamrng.c b/drivers/crypto/caam/caamrng.c
index 7fbda1b08360..e8baacaabe07 100644
--- a/drivers/crypto/caam/caamrng.c
+++ b/drivers/crypto/caam/caamrng.c
@@ -53,7 +53,7 @@
 L1_CACHE_BYTES)
 
 /* length of descriptors */
-#define DESC_JOB_O_LEN (CAAM_CMD_SZ * 2 + CAAM_PTR_SZ * 2)
+#define DESC_JOB_O_LEN (CAAM_CMD_SZ * 2 + CAAM_PTR_SZ_MAX * 2)
 #define DESC_RNG_LEN   (3 * CAAM_CMD_SZ)
 
 /* Buffer, its dma address and lock */
diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c
index 0b4007068c31..47b92451756f 100644
--- a/drivers/crypto/caam/ctrl.c
+++ b/drivers/crypto/caam/ctrl.c
@@ -602,6 +602,7 @@ static int caam_probe(struct platform_device *pdev)
caam_imx = (bool)imx_soc_match;
 
comp_params = rd_reg32(>perfmon.comp_parms_ms);
+   caam_ptr_sz = sizeof(dma_addr_t);
caam_dpaa2 = !!(comp_params & CTPR_MS_DPAA2);
ctrlpriv->qi_present = !!(comp_params & CTPR_MS_QI_MASK);
 
diff --git a/drivers/crypto/caam/desc_constr.h 
b/drivers/crypto/caam/desc_constr.h
index 1fe50a4fefaa..89187831d74f 100644
--- a/drivers/crypto/caam/desc_constr.h
+++ b/drivers/crypto/caam/desc_constr.h
@@ -14,9 +14,14 @@
 
 #define IMMEDIATE (1 << 23)
 #define CAAM_CMD_SZ sizeof(u32)
-#define CAAM_PTR_SZ sizeof(dma_addr_t)
+#define CAAM_PTR_SZ caam_ptr_sz
+#define CAAM_PTR_SZ_MAX sizeof(dma_addr_t)
+#define CAAM_PTR_SZ_MIN sizeof(u32)
 #define CAAM_DESC_BYTES_MAX (CAAM_CMD_SZ * MAX_CAAM_DESCSIZE)
-#define DESC_JOB_IO_LEN (CAAM_CMD_SZ * 5 + CAAM_PTR_SZ * 3)
+#define __DESC_JOB_IO_LEN(n) (CAAM_CMD_SZ * 5 + (n) * 3)
+#define DESC_JOB_IO_LEN __DESC_JOB_IO_LEN(CAAM_PTR_SZ)
+#define DESC_JOB_IO_LEN_MAX __DESC_JOB_IO_LEN(CAAM_PTR_SZ_MAX)
+#define DESC_JOB_IO_LEN_MIN __DESC_JOB_IO_LEN(CAAM_PTR_SZ_MIN)
 
 /*
  * The CAAM QI hardware constructs a job descriptor which points
@@ -43,7 +48,7 @@
  * get loaded in deco buffer are '8' or '11'. The remaining words
  * in deco buffer can be used for storing shared descriptor.
  */
-#define MAX_SDLEN  ((CAAM_DESC_BYTES_MAX - DESC_JOB_IO_LEN) / CAAM_CMD_SZ)
+#define MAX_SDLEN  ((CAAM_DESC_BYTES_MAX - DESC_JOB_IO_LEN_MIN) / 
CAAM_CMD_SZ)
 
 #ifdef DEBUG
 #define PRINT_POS do { printk(KERN_DEBUG "%02d: %s\n", desc_len(desc),\
@@ -64,6 +69,7 @@
   (LDOFF_ENABLE_AUTO_NFIFO << LDST_OFFSET_SHIFT))
 
 extern bool caam_little_end;
+extern size_t caam_ptr_sz;
 
 /*
  * HW fetches 4 S/G table entries at a time, irrespective of how many entries
diff --git a/drivers/crypto/caam/error.c b/drivers/crypto/caam/error.c
index b7fbf1be37a4..17c6108b6d41 100644
--- a/drivers/crypto/caam/error.c
+++ b/drivers/crypto/caam/error.c
@@ -56,6 +56,9 @@ EXPORT_SYMBOL(caam_little_end);
 bool caam_imx;
 EXPORT_SYMBOL(caam_imx);
 
+size_t caam_ptr_sz;
+EXPORT_SYMBOL(caam_ptr_sz);
+
 static const struct {
u8 value;
const char *error_text;
-- 
2.21.0

[PATCH v8 04/16] crypto: caam - request JR IRQ as the last step

[Andrey Smirnov]

In order to avoid any risk of JR IRQ request being handled while some
of the resources used for that are not yet allocated move the code
requesting said IRQ to the endo of caam_jr_init().

Signed-off-by: Andrey Smirnov 
Cc: Chris Spencer 
Cc: Cory Tusar 
Cc: Chris Healy 
Cc: Lucas Stach 
Cc: Horia Geantă 
Cc: Aymen Sghaier 
Cc: Leonard Crestez 
Cc: linux-cry...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/crypto/caam/jr.c | 34 +++---
 1 file changed, 15 insertions(+), 19 deletions(-)

diff --git a/drivers/crypto/caam/jr.c b/drivers/crypto/caam/jr.c
index ea02f7774f7c..98b308de42c0 100644
--- a/drivers/crypto/caam/jr.c
+++ b/drivers/crypto/caam/jr.c
@@ -428,38 +428,26 @@ static int caam_jr_init(struct device *dev)
 
jrp = dev_get_drvdata(dev);
 
-   tasklet_init(>irqtask, caam_jr_dequeue, (unsigned long)dev);
-
-   /* Connect job ring interrupt handler. */
-   error = devm_request_irq(dev, jrp->irq, caam_jr_interrupt, IRQF_SHARED,
-dev_name(dev), dev);
-   if (error) {
-   dev_err(dev, "can't connect JobR %d interrupt (%d)\n",
-   jrp->ridx, jrp->irq);
-   goto out_kill_deq;
-   }
-
error = caam_reset_hw_jr(dev);
if (error)
-   goto out_kill_deq;
+   return error;
 
-   error = -ENOMEM;
jrp->inpring = dmam_alloc_coherent(dev, sizeof(*jrp->inpring) *
   JOBR_DEPTH, ,
   GFP_KERNEL);
if (!jrp->inpring)
-   goto out_kill_deq;
+   return -ENOMEM;
 
jrp->outring = dmam_alloc_coherent(dev, sizeof(*jrp->outring) *
   JOBR_DEPTH, ,
   GFP_KERNEL);
if (!jrp->outring)
-   goto out_kill_deq;
+   return -ENOMEM;
 
jrp->entinfo = devm_kcalloc(dev, JOBR_DEPTH, sizeof(*jrp->entinfo),
GFP_KERNEL);
if (!jrp->entinfo)
-   goto out_kill_deq;
+   return -ENOMEM;
 
for (i = 0; i < JOBR_DEPTH; i++)
jrp->entinfo[i].desc_addr_dma = !0;
@@ -483,9 +471,17 @@ static int caam_jr_init(struct device *dev)
  (JOBR_INTC_COUNT_THLD << JRCFG_ICDCT_SHIFT) |
  (JOBR_INTC_TIME_THLD << JRCFG_ICTT_SHIFT));
 
-   return 0;
-out_kill_deq:
-   tasklet_kill(>irqtask);
+   tasklet_init(>irqtask, caam_jr_dequeue, (unsigned long)dev);
+
+   /* Connect job ring interrupt handler. */
+   error = devm_request_irq(dev, jrp->irq, caam_jr_interrupt, IRQF_SHARED,
+dev_name(dev), dev);
+   if (error) {
+   dev_err(dev, "can't connect JobR %d interrupt (%d)\n",
+   jrp->ridx, jrp->irq);
+   tasklet_kill(>irqtask);
+   }
+
return error;
 }
 
-- 
2.21.0

[PATCH v8 07/16] crypto: caam - drop 64-bit only wr/rd_reg64()

[Andrey Smirnov]

Since 32-bit of both wr_reg64 and rd_reg64 now use 64-bit IO helpers,
these functions should no longer be necessary. No functional change intended.

Signed-off-by: Andrey Smirnov 
Reviewed-by: Horia Geantă 
Cc: Chris Spencer 
Cc: Cory Tusar 
Cc: Chris Healy 
Cc: Lucas Stach 
Cc: Horia Geantă 
Cc: Aymen Sghaier 
Cc: Leonard Crestez 
Cc: linux-cry...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/crypto/caam/regs.h | 19 ---
 1 file changed, 19 deletions(-)

diff --git a/drivers/crypto/caam/regs.h b/drivers/crypto/caam/regs.h
index 4efc10534873..489d6c1eec7d 100644
--- a/drivers/crypto/caam/regs.h
+++ b/drivers/crypto/caam/regs.h
@@ -138,24 +138,6 @@ static inline void clrsetbits_32(void __iomem *reg, u32 
clear, u32 set)
  *base + 0x : least-significant 32 bits
  *base + 0x0004 : most-significant 32 bits
  */
-#ifdef CONFIG_64BIT
-static inline void wr_reg64(void __iomem *reg, u64 data)
-{
-   if (caam_little_end)
-   iowrite64(data, reg);
-   else
-   iowrite64be(data, reg);
-}
-
-static inline u64 rd_reg64(void __iomem *reg)
-{
-   if (caam_little_end)
-   return ioread64(reg);
-   else
-   return ioread64be(reg);
-}
-
-#else /* CONFIG_64BIT */
 static inline void wr_reg64(void __iomem *reg, u64 data)
 {
if (caam_little_end) {
@@ -187,7 +169,6 @@ static inline u64 rd_reg64(void __iomem *reg)
return ioread64be(reg);
}
 }
-#endif /* CONFIG_64BIT  */
 
 static inline u64 cpu_to_caam_dma64(dma_addr_t value)
 {
-- 
2.21.0

[PATCH v8 01/16] crypto: caam - move DMA mask selection into a function

[Andrey Smirnov]

Exactly the same code to figure out DMA mask is repeated twice in the
driver code. To avoid repetition, move that logic into a standalone
subroutine in intern.h. While at it re-shuffle the code to make it
more readable with early returns.

Signed-off-by: Andrey Smirnov 
Reviewed-by: Horia Geantă 
Cc: Chris Spencer 
Cc: Cory Tusar 
Cc: Chris Healy 
Cc: Lucas Stach 
Cc: Horia Geantă 
Cc: Aymen Sghaier 
Cc: Leonard Crestez 
Cc: linux-cry...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/crypto/caam/ctrl.c   | 11 +--
 drivers/crypto/caam/intern.h | 20 
 drivers/crypto/caam/jr.c | 15 +--
 3 files changed, 22 insertions(+), 24 deletions(-)

diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c
index e0590beae240..50336494f285 100644
--- a/drivers/crypto/caam/ctrl.c
+++ b/drivers/crypto/caam/ctrl.c
@@ -711,16 +711,7 @@ static int caam_probe(struct platform_device *pdev)
  JRSTART_JR1_START | JRSTART_JR2_START |
  JRSTART_JR3_START);
 
-   if (sizeof(dma_addr_t) == sizeof(u64)) {
-   if (caam_dpaa2)
-   ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(49));
-   else if (of_device_is_compatible(nprop, "fsl,sec-v5.0"))
-   ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(40));
-   else
-   ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(36));
-   } else {
-   ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
-   }
+   ret = dma_set_mask_and_coherent(dev, caam_get_dma_mask(dev));
if (ret) {
dev_err(dev, "dma_set_mask_and_coherent failed (%d)\n", ret);
goto disable_caam_emi_slow;
diff --git a/drivers/crypto/caam/intern.h b/drivers/crypto/caam/intern.h
index 6af84bbc612c..ec25d260fa40 100644
--- a/drivers/crypto/caam/intern.h
+++ b/drivers/crypto/caam/intern.h
@@ -10,6 +10,8 @@
 #ifndef INTERN_H
 #define INTERN_H
 
+#include "ctrl.h"
+
 /* Currently comes from Kconfig param as a ^2 (driver-required) */
 #define JOBR_DEPTH (1 << CONFIG_CRYPTO_DEV_FSL_CAAM_RINGSIZE)
 
@@ -215,4 +217,22 @@ DEFINE_SIMPLE_ATTRIBUTE(caam_fops_u32_ro, 
caam_debugfs_u32_get, NULL, "%llu\n");
 DEFINE_SIMPLE_ATTRIBUTE(caam_fops_u64_ro, caam_debugfs_u64_get, NULL, 
"%llu\n");
 #endif
 
+static inline u64 caam_get_dma_mask(struct device *dev)
+{
+   struct device_node *nprop = dev->of_node;
+
+   if (sizeof(dma_addr_t) != sizeof(u64))
+   return DMA_BIT_MASK(32);
+
+   if (caam_dpaa2)
+   return DMA_BIT_MASK(49);
+
+   if (of_device_is_compatible(nprop, "fsl,sec-v5.0-job-ring") ||
+   of_device_is_compatible(nprop, "fsl,sec-v5.0"))
+   return DMA_BIT_MASK(40);
+
+   return DMA_BIT_MASK(36);
+}
+
+
 #endif /* INTERN_H */
diff --git a/drivers/crypto/caam/jr.c b/drivers/crypto/caam/jr.c
index cea811fed320..4b25b2fa3d02 100644
--- a/drivers/crypto/caam/jr.c
+++ b/drivers/crypto/caam/jr.c
@@ -543,20 +543,7 @@ static int caam_jr_probe(struct platform_device *pdev)
 
jrpriv->rregs = (struct caam_job_ring __iomem __force *)ctrl;
 
-   if (sizeof(dma_addr_t) == sizeof(u64)) {
-   if (caam_dpaa2)
-   error = dma_set_mask_and_coherent(jrdev,
- DMA_BIT_MASK(49));
-   else if (of_device_is_compatible(nprop,
-"fsl,sec-v5.0-job-ring"))
-   error = dma_set_mask_and_coherent(jrdev,
- DMA_BIT_MASK(40));
-   else
-   error = dma_set_mask_and_coherent(jrdev,
- DMA_BIT_MASK(36));
-   } else {
-   error = dma_set_mask_and_coherent(jrdev, DMA_BIT_MASK(32));
-   }
+   error = dma_set_mask_and_coherent(jrdev, caam_get_dma_mask(jrdev));
if (error) {
dev_err(jrdev, "dma_set_mask_and_coherent failed (%d)\n",
error);
-- 
2.21.0

[PATCH] ARM: dts: vf610-zii-dev-rev-b: Drop redundant I2C properties

[Andrey Smirnov]

Drop redundant I2C properties that are already specified in
vf610-zii-dev.dtsi

Signed-off-by: Andrey Smirnov 
Cc: Shawn Guo 
Cc: Chris Healy 
Cc: Fabio Estevam 
Cc: linux-arm-ker...@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
---
 arch/arm/boot/dts/vf610-zii-dev-rev-b.dts | 10 --
 1 file changed, 10 deletions(-)

diff --git a/arch/arm/boot/dts/vf610-zii-dev-rev-b.dts 
b/arch/arm/boot/dts/vf610-zii-dev-rev-b.dts
index 48086c5e8549..e500911ce0a5 100644
--- a/arch/arm/boot/dts/vf610-zii-dev-rev-b.dts
+++ b/arch/arm/boot/dts/vf610-zii-dev-rev-b.dts
@@ -323,11 +323,6 @@
 };
 
  {
-   clock-frequency = <10>;
-   pinctrl-names = "default";
-   pinctrl-0 = <_i2c0>;
-   status = "okay";
-
gpio5: io-expander@20 {
compatible = "nxp,pca9554";
reg = <0x20>;
@@ -350,11 +345,6 @@
 };
 
  {
-   clock-frequency = <10>;
-   pinctrl-names = "default";
-   pinctrl-0 = <_i2c2>;
-   status = "okay";
-
tca9548@70 {
compatible = "nxp,pca9548";
pinctrl-0 = <_i2c_mux_reset>;
-- 
2.21.0

[PATCH] ARM: dts: vf610-zii-scu4-aib: Drop "rs485-rts-delay" property

[Andrey Smirnov]

LPUART driver does not support specifying "rs485-rts-delay"
property. Drop it.

Signed-off-by: Andrey Smirnov 
Cc: Shawn Guo 
Cc: Chris Healy 
Cc: Fabio Estevam 
Cc: linux-arm-ker...@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
---
 arch/arm/boot/dts/vf610-zii-scu4-aib.dts | 2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/arm/boot/dts/vf610-zii-scu4-aib.dts 
b/arch/arm/boot/dts/vf610-zii-scu4-aib.dts
index 666ec27a73e3..d8c38ef6a98a 100644
--- a/arch/arm/boot/dts/vf610-zii-scu4-aib.dts
+++ b/arch/arm/boot/dts/vf610-zii-scu4-aib.dts
@@ -685,7 +685,6 @@
linux,rs485-enabled-at-boot-time;
pinctrl-names = "default";
pinctrl-0 = <_uart1>;
-   rs485-rts-delay = <0 200>;
status = "okay";
 };
 
@@ -693,7 +692,6 @@
linux,rs485-enabled-at-boot-time;
pinctrl-names = "default";
pinctrl-0 = <_uart2>;
-   rs485-rts-delay = <0 200>;
status = "okay";
 };
 
-- 
2.21.0

[PATCH] ARM: dts: vf610-zii-cfu1: Slow I2C0 down to 100kHz

[Andrey Smirnov]

Fiber-optic module attached to the bus is only rated to work at
100kHz, so drop the bus frequncy to accomodate that.

Signed-off-by: Andrey Smirnov 
Cc: Shawn Guo 
Cc: Chris Healy 
Cc: Fabio Estevam 
Cc: linux-arm-ker...@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
---
 arch/arm/boot/dts/vf610-zii-cfu1.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/vf610-zii-cfu1.dts 
b/arch/arm/boot/dts/vf610-zii-cfu1.dts
index ff460a1de85a..28732249cfc0 100644
--- a/arch/arm/boot/dts/vf610-zii-cfu1.dts
+++ b/arch/arm/boot/dts/vf610-zii-cfu1.dts
@@ -207,7 +207,7 @@
 };
 
  {
-   clock-frequency = <40>;
+   clock-frequency = <10>;
pinctrl-names = "default";
pinctrl-0 = <_i2c0>;
status = "okay";
-- 
2.21.0

[PATCH] ARM: vf610-zii-cfu1: Add node for switch watchdog

[Andrey Smirnov]

Add I2C child node for switch watchdog present on CFU1.

Signed-off-by: Andrey Smirnov 
Signed-off-by: Cory Tusar 
Cc: Shawn Guo 
Cc: Chris Healy 
Cc: Fabio Estevam 
Cc: linux-arm-ker...@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
---
 arch/arm/boot/dts/vf610-zii-cfu1.dts | 19 +++
 1 file changed, 19 insertions(+)

diff --git a/arch/arm/boot/dts/vf610-zii-cfu1.dts 
b/arch/arm/boot/dts/vf610-zii-cfu1.dts
index 7267873b5369..18c19c092dd1 100644
--- a/arch/arm/boot/dts/vf610-zii-cfu1.dts
+++ b/arch/arm/boot/dts/vf610-zii-cfu1.dts
@@ -239,6 +239,18 @@
};
 };
 
+ {
+   clock-frequency = <10>;
+   pinctrl-names = "default";
+   pinctrl-0 = <_i2c1>;
+   status = "okay";
+
+   watchdog@38 {
+   compatible = "zii,rave-wdt";
+   reg = <0x38>;
+   };
+};
+
  {
status = "disabled";
 };
@@ -324,6 +336,13 @@
>;
};
 
+   pinctrl_i2c1: i2c1grp {
+   fsl,pins = <
+   VF610_PAD_PTB16__I2C1_SCL   0x37ff
+   VF610_PAD_PTB17__I2C1_SDA   0x37ff
+   >;
+   };
+
pinctrl_leds_debug: pinctrl-leds-debug {
fsl,pins = <
VF610_PAD_PTD3__GPIO_82 0x31c2
-- 
2.21.0

[PATCH v2 06/22] watchdog: ziirave_wdt: Simplify ziirave_firm_write_pkt()

[Andrey Smirnov]

There no reason why ziirave_firm_write_pkt() has to take firmware
data via 'struct ihex_binrec' and it can just take address, data pointer
and data length as individual arguments. Make this change to allow us
to drastically simplify handling page crossing case by removing all of
the extra code required to split 'struct ihex_binrec' into two.

Signed-off-by: Andrey Smirnov 
Cc: Chris Healy 
Cc: Guenter Roeck 
Cc: Rick Ramstetter 
Cc: linux-watch...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/watchdog/ziirave_wdt.c | 116 +
 1 file changed, 44 insertions(+), 72 deletions(-)

diff --git a/drivers/watchdog/ziirave_wdt.c b/drivers/watchdog/ziirave_wdt.c
index 75c066602c00..b2d5ff0c22fe 100644
--- a/drivers/watchdog/ziirave_wdt.c
+++ b/drivers/watchdog/ziirave_wdt.c
@@ -51,6 +51,7 @@ static char *ziirave_reasons[] = {"power cycle", "hw 
watchdog", NULL, NULL,
 #define ZIIRAVE_FIRM_PKT_DATA_SIZE 16
 #define ZIIRAVE_FIRM_FLASH_MEMORY_START0x1600
 #define ZIIRAVE_FIRM_FLASH_MEMORY_END  0x2bbf
+#define ZIIRAVE_FIRM_PAGE_SIZE 128
 
 /* Received and ready for next Download packet. */
 #define ZIIRAVE_FIRM_DOWNLOAD_ACK  1
@@ -244,27 +245,26 @@ static int ziirave_firm_write_byte(struct watchdog_device 
*wdd, u8 command,
  * Data0 .. Data15: Array of 16 bytes of data.
  * Checksum: Checksum byte to verify data integrity.
  */
-static int ziirave_firm_write_pkt(struct watchdog_device *wdd,
- const struct ihex_binrec *rec)
+static int __ziirave_firm_write_pkt(struct watchdog_device *wdd,
+   u32 addr, const u8 *data, u8 len)
 {
+   const u16 addr16 = (u16)addr / 2;
struct i2c_client *client = to_i2c_client(wdd->parent);
u8 i, checksum = 0, packet[ZIIRAVE_FIRM_PKT_TOTAL_SIZE];
int ret;
-   u16 addr;
 
memset(packet, 0, ARRAY_SIZE(packet));
 
/* Packet length */
-   packet[0] = (u8)be16_to_cpu(rec->len);
+   packet[0] = len;
/* Packet address */
-   addr = (be32_to_cpu(rec->addr) & 0x) >> 1;
-   packet[1] = addr & 0xff;
-   packet[2] = (addr & 0xff00) >> 8;
+   packet[1] = addr16 & 0xff;
+   packet[2] = (addr16 & 0xff00) >> 8;
 
/* Packet data */
-   if (be16_to_cpu(rec->len) > ZIIRAVE_FIRM_PKT_DATA_SIZE)
+   if (len > ZIIRAVE_FIRM_PKT_DATA_SIZE)
return -EMSGSIZE;
-   memcpy(packet + 3, rec->data, be16_to_cpu(rec->len));
+   memcpy(packet + 3, data, len);
 
/* Packet checksum */
for (i = 0; i < ZIIRAVE_FIRM_PKT_TOTAL_SIZE - 1; i++)
@@ -276,11 +276,35 @@ static int ziirave_firm_write_pkt(struct watchdog_device 
*wdd,
if (ret)
dev_err(>dev,
  "Failed to write firmware packet at address 0x%04x: %d\n",
- addr, ret);
+ addr16, ret);
 
return ret;
 }
 
+static int ziirave_firm_write_pkt(struct watchdog_device *wdd,
+ u32 addr, const u8 *data, u8 len)
+{
+   const u8 max_write_len = ZIIRAVE_FIRM_PAGE_SIZE -
+   (addr - ALIGN_DOWN(addr, ZIIRAVE_FIRM_PAGE_SIZE));
+   int ret;
+
+   if (len > max_write_len) {
+   /*
+* If data crossed page boundary we need to split this
+* write in two
+*/
+   ret = __ziirave_firm_write_pkt(wdd, addr, data, max_write_len);
+   if (ret)
+   return ret;
+
+   addr += max_write_len;
+   data += max_write_len;
+   len  -= max_write_len;
+   }
+
+   return __ziirave_firm_write_pkt(wdd, addr, data, len);
+}
+
 static int ziirave_firm_verify(struct watchdog_device *wdd,
   const struct firmware *fw)
 {
@@ -333,9 +357,8 @@ static int ziirave_firm_upload(struct watchdog_device *wdd,
   const struct firmware *fw)
 {
struct i2c_client *client = to_i2c_client(wdd->parent);
-   int ret, words_till_page_break;
const struct ihex_binrec *rec;
-   struct ihex_binrec *rec_new;
+   int ret;
 
ret = ziirave_firm_write_byte(wdd, ZIIRAVE_CMD_JUMP_TO_BOOTLOADER, 1,
  false);
@@ -366,68 +389,17 @@ static int ziirave_firm_upload(struct watchdog_device 
*wdd,
return -EMSGSIZE;
}
 
-   /* Calculate words till page break */
-   words_till_page_break = (64 - ((be32_to_cpu(rec->addr) >> 1) &
-0x3f));
-   if ((be16_to_cpu(rec->len) >> 1) > words_till_page_break) {
-   /*
-* Data in passes page boundary, so we need t

[PATCH v2 17/22] watchdog: ziirave_wdt: Fix DOWNLOAD_END payload

[Andrey Smirnov]

Bootloader firmware expects the following traffic for DOWNLOAD_END:

S Addr Wr [A] 0x11 [A] P

using ziirave_firm_write_byte() will result in

S Addr Wr [A] 0x11 [A] 0x01 [A] 0x01 [A] P

which happens to work because firmware will ignore any extra bytes
sent. Fix this by converting the code to use i2c_smbus_write_byte()
instead.

Signed-off-by: Andrey Smirnov 
Cc: Chris Healy 
Cc: Guenter Roeck 
Cc: Rick Ramstetter 
Cc: linux-watch...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/watchdog/ziirave_wdt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/watchdog/ziirave_wdt.c b/drivers/watchdog/ziirave_wdt.c
index 0185b9175cc0..a598780d73d3 100644
--- a/drivers/watchdog/ziirave_wdt.c
+++ b/drivers/watchdog/ziirave_wdt.c
@@ -425,7 +425,7 @@ static int ziirave_firm_upload(struct watchdog_device *wdd,
}
 
/* End download operation */
-   ret = ziirave_firm_write_byte(wdd, ZIIRAVE_CMD_DOWNLOAD_END, 1, false);
+   ret = i2c_smbus_write_byte(client, ZIIRAVE_CMD_DOWNLOAD_END);
if (ret) {
dev_err(>dev,
"Failed to end firmware download: %d\n", ret);
-- 
2.21.0

[PATCH v2 03/22] watchdog: ziirave_wdt: Be more verbose during firmware update

[Andrey Smirnov]

Add more error logging to ziirave_firm_upload() for diagnostics.

Signed-off-by: Andrey Smirnov 
Reviewed-by: Guenter Roeck 
Cc: Chris Healy 
Cc: Guenter Roeck 
Cc: Rick Ramstetter 
Cc: linux-watch...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/watchdog/ziirave_wdt.c | 18 ++
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/watchdog/ziirave_wdt.c b/drivers/watchdog/ziirave_wdt.c
index 8c71341a9c1c..b3e255b40209 100644
--- a/drivers/watchdog/ziirave_wdt.c
+++ b/drivers/watchdog/ziirave_wdt.c
@@ -335,14 +335,18 @@ static int ziirave_firm_upload(struct watchdog_device 
*wdd,
 
ret = ziirave_firm_write_byte(wdd, ZIIRAVE_CMD_JUMP_TO_BOOTLOADER, 1,
  false);
-   if (ret)
+   if (ret) {
+   dev_err(>dev, "Failed to jump to bootloader\n");
return ret;
+   }
 
msleep(500);
 
ret = ziirave_firm_write_byte(wdd, ZIIRAVE_CMD_DOWNLOAD_START, 1, true);
-   if (ret)
+   if (ret) {
+   dev_err(>dev, "Failed to start download\n");
return ret;
+   }
 
msleep(500);
 
@@ -438,14 +442,20 @@ static int ziirave_firm_upload(struct watchdog_device 
*wdd,
 
/* End download operation */
ret = ziirave_firm_write_byte(wdd, ZIIRAVE_CMD_DOWNLOAD_END, 1, false);
-   if (ret)
+   if (ret) {
+   dev_err(>dev,
+   "Failed to end firmware download: %d\n", ret);
return ret;
+   }
 
/* Reset the processor */
ret = ziirave_firm_write_byte(wdd, ZIIRAVE_CMD_RESET_PROCESSOR, 1,
  false);
-   if (ret)
+   if (ret) {
+   dev_err(>dev,
+   "Failed to reset the watchdog: %d\n", ret);
return ret;
+   }
 
msleep(500);
 
-- 
2.21.0

[PATCH v2 04/22] watchdog: ziirave_wdt: Don't bail out on unexpected timeout value

[Andrey Smirnov]

Reprogramming bootloader on watchdog MCU will result in reported
default timeout value of "0". That in turn will be unnecessarily
rejected by the driver as invalid device (-ENODEV). Simplify probe to
read stored timeout value, set it to a sane default if it is bogus,
and then program that value unconditionally.

Signed-off-by: Andrey Smirnov 
Cc: Chris Healy 
Cc: Guenter Roeck 
Cc: Rick Ramstetter 
Cc: linux-watch...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/watchdog/ziirave_wdt.c | 22 +++---
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/watchdog/ziirave_wdt.c b/drivers/watchdog/ziirave_wdt.c
index b3e255b40209..a11b92383c5f 100644
--- a/drivers/watchdog/ziirave_wdt.c
+++ b/drivers/watchdog/ziirave_wdt.c
@@ -23,6 +23,7 @@
 
 #define ZIIRAVE_TIMEOUT_MIN3
 #define ZIIRAVE_TIMEOUT_MAX255
+#define ZIIRAVE_TIMEOUT_DEFAULT30
 
 #define ZIIRAVE_PING_VALUE 0x0
 
@@ -673,22 +674,21 @@ static int ziirave_wdt_probe(struct i2c_client *client,
return val;
}
 
-   if (val < ZIIRAVE_TIMEOUT_MIN)
-   return -ENODEV;
+   if (val > ZIIRAVE_TIMEOUT_MAX ||
+   val < ZIIRAVE_TIMEOUT_MIN)
+   val = ZIIRAVE_TIMEOUT_DEFAULT;
 
w_priv->wdd.timeout = val;
-   } else {
-   ret = ziirave_wdt_set_timeout(_priv->wdd,
- w_priv->wdd.timeout);
-   if (ret) {
-   dev_err(>dev, "Failed to set timeout\n");
-   return ret;
-   }
+   }
 
-   dev_info(>dev, "Timeout set to %ds\n",
-w_priv->wdd.timeout);
+   ret = ziirave_wdt_set_timeout(_priv->wdd, w_priv->wdd.timeout);
+   if (ret) {
+   dev_err(>dev, "Failed to set timeout\n");
+   return ret;
}
 
+   dev_info(>dev, "Timeout set to %ds\n", w_priv->wdd.timeout);
+
watchdog_set_nowayout(_priv->wdd, nowayout);
 
i2c_set_clientdata(client, w_priv);
-- 
2.21.0

[PATCH v2 18/22] watchdog: ziirave_wdt: Fix RESET_PROCESSOR payload

[Andrey Smirnov]

Bootloader firmware expects the following traffic for
RESET_PROCESSOR:

S Addr Wr [A] 0x0b [A] 0x01 [A] P

using ziirave_firm_write_byte() will result in

S Addr Wr [A] 0x0b [A] 0x01 [A] 0x01 [A] P

which happens to work because firmware will ignore any extra bytes and
expected magic value matches byte count sent by
i2c_smbus_write_block_data(). Fix this by converting the code to use
i2c_smbus_write_byte_data() instead.

Signed-off-by: Andrey Smirnov 
Cc: Chris Healy 
Cc: Guenter Roeck 
Cc: Rick Ramstetter 
Cc: linux-watch...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/watchdog/ziirave_wdt.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/watchdog/ziirave_wdt.c b/drivers/watchdog/ziirave_wdt.c
index a598780d73d3..92df27350e41 100644
--- a/drivers/watchdog/ziirave_wdt.c
+++ b/drivers/watchdog/ziirave_wdt.c
@@ -73,6 +73,7 @@ static char *ziirave_reasons[] = {"power cycle", "hw 
watchdog", NULL, NULL,
 #define ZIIRAVE_CMD_DOWNLOAD_PACKET0x0e
 
 #define ZIIRAVE_CMD_JUMP_TO_BOOTLOADER_MAGIC   1
+#define ZIIRAVE_CMD_RESET_PROCESSOR_MAGIC  1
 
 #define ZIIRAVE_FW_VERSION_FMT "02.%02u.%02u"
 #define ZIIRAVE_BL_VERSION_FMT "01.%02u.%02u"
@@ -433,8 +434,9 @@ static int ziirave_firm_upload(struct watchdog_device *wdd,
}
 
/* Reset the processor */
-   ret = ziirave_firm_write_byte(wdd, ZIIRAVE_CMD_RESET_PROCESSOR, 1,
- false);
+   ret = i2c_smbus_write_byte_data(client,
+   ZIIRAVE_CMD_RESET_PROCESSOR,
+   ZIIRAVE_CMD_RESET_PROCESSOR_MAGIC);
if (ret) {
dev_err(>dev,
"Failed to reset the watchdog: %d\n", ret);
-- 
2.21.0

[PATCH v2 10/22] watchdog: ziirave_wdt: Zero out only what's necessary

[Andrey Smirnov]

Instead of zeroing out all of the packet and then overwriting a
significant portion of those zeros via memcpy(), zero out only a
portion of the packet that is known to not contain any data.

Signed-off-by: Andrey Smirnov 
Cc: Chris Healy 
Cc: Guenter Roeck 
Cc: Rick Ramstetter 
Cc: linux-watch...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/watchdog/ziirave_wdt.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/watchdog/ziirave_wdt.c b/drivers/watchdog/ziirave_wdt.c
index e0f55cbdc603..69694f2836d7 100644
--- a/drivers/watchdog/ziirave_wdt.c
+++ b/drivers/watchdog/ziirave_wdt.c
@@ -260,8 +260,6 @@ static int __ziirave_firm_write_pkt(struct watchdog_device 
*wdd,
return -EMSGSIZE;
}
 
-   memset(packet, 0, sizeof(packet));
-
/* Packet length */
packet[0] = len;
/* Packet address */
@@ -269,6 +267,7 @@ static int __ziirave_firm_write_pkt(struct watchdog_device 
*wdd,
packet[2] = (addr16 & 0xff00) >> 8;
 
memcpy(packet + 3, data, len);
+   memset(packet + 3 + len, 0, ZIIRAVE_FIRM_PKT_DATA_SIZE - len);
 
/* Packet checksum */
for (i = 0; i < len + 3; i++)
-- 
2.21.0

[PATCH v2 13/22] watchdog: ziirave_wdt: Don't read out more than 'len' firmware bytes

[Andrey Smirnov]

We only compare first 'len' bytes of read firmware, so we don't need
to read more that that.

Signed-off-by: Andrey Smirnov 
Cc: Chris Healy 
Cc: Guenter Roeck 
Cc: Rick Ramstetter 
Cc: linux-watch...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/watchdog/ziirave_wdt.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/watchdog/ziirave_wdt.c b/drivers/watchdog/ziirave_wdt.c
index 4b95467bf239..f05095b08016 100644
--- a/drivers/watchdog/ziirave_wdt.c
+++ b/drivers/watchdog/ziirave_wdt.c
@@ -318,6 +318,8 @@ static int ziirave_firm_verify(struct watchdog_device *wdd,
u16 addr;
 
for (rec = (void *)fw->data; rec; rec = ihex_next_binrec(rec)) {
+   const u16 len = be16_to_cpu(rec->len);
+
addr = (be32_to_cpu(rec->addr) & 0x) >> 1;
if (addr < ZIIRAVE_FIRM_FLASH_MEMORY_START ||
addr > ZIIRAVE_FIRM_FLASH_MEMORY_END)
@@ -331,7 +333,7 @@ static int ziirave_firm_verify(struct watchdog_device *wdd,
return ret;
}
 
-   for (i = 0; i < ARRAY_SIZE(data); i++) {
+   for (i = 0; i < len; i++) {
ret = i2c_smbus_read_byte_data(client,
ZIIRAVE_CMD_DOWNLOAD_READ_BYTE);
if (ret < 0) {
@@ -342,7 +344,7 @@ static int ziirave_firm_verify(struct watchdog_device *wdd,
data[i] = ret;
}
 
-   if (memcmp(data, rec->data, be16_to_cpu(rec->len))) {
+   if (memcmp(data, rec->data, len)) {
dev_err(>dev,
"Firmware mismatch at address 0x%04x\n", addr);
return -EINVAL;
-- 
2.21.0

[PATCH v2 16/22] watchdog: ziirave_wdt: Fix JUMP_TO_BOOTLOADER payload

[Andrey Smirnov]

Bootloader firmware expects the following traffic for
JUMP_TO_BOOTLOADER:

S Addr Wr [A] 0x0c [A] 0x01 [A] P

using ziirave_firm_write_byte() will result in

S Addr Wr [A] 0x0c [A] 0x01 [A] 0x01 [A] P

which happens to work because firmware will ignore any extra bytes and
expected magic value matches byte count sent by
i2c_smbus_write_block_data(). Fix this by converting the code to use
i2c_smbus_write_byte_data() instead.

Signed-off-by: Andrey Smirnov 
Cc: Chris Healy 
Cc: Guenter Roeck 
Cc: Rick Ramstetter 
Cc: linux-watch...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/watchdog/ziirave_wdt.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/watchdog/ziirave_wdt.c b/drivers/watchdog/ziirave_wdt.c
index 0c150f3cf408..0185b9175cc0 100644
--- a/drivers/watchdog/ziirave_wdt.c
+++ b/drivers/watchdog/ziirave_wdt.c
@@ -72,6 +72,8 @@ static char *ziirave_reasons[] = {"power cycle", "hw 
watchdog", NULL, NULL,
 #define ZIIRAVE_CMD_JUMP_TO_BOOTLOADER 0x0c
 #define ZIIRAVE_CMD_DOWNLOAD_PACKET0x0e
 
+#define ZIIRAVE_CMD_JUMP_TO_BOOTLOADER_MAGIC   1
+
 #define ZIIRAVE_FW_VERSION_FMT "02.%02u.%02u"
 #define ZIIRAVE_BL_VERSION_FMT "01.%02u.%02u"
 
@@ -376,8 +378,9 @@ static int ziirave_firm_upload(struct watchdog_device *wdd,
const struct ihex_binrec *rec;
int ret;
 
-   ret = ziirave_firm_write_byte(wdd, ZIIRAVE_CMD_JUMP_TO_BOOTLOADER, 1,
- false);
+   ret = i2c_smbus_write_byte_data(client,
+   ZIIRAVE_CMD_JUMP_TO_BOOTLOADER,
+   ZIIRAVE_CMD_JUMP_TO_BOOTLOADER_MAGIC);
if (ret) {
dev_err(>dev, "Failed to jump to bootloader\n");
return ret;
-- 
2.21.0

[PATCH v2 14/22] watchdog: ziirave_wdt: Don't try to program readonly flash

[Andrey Smirnov]

Bootloader code will ignore any attempts to write data to any flash
area outside of [ZIIRAVE_FIRM_FLASH_MEMORY_START;
ZIIRAVE_FIRM_FLASH_MEMORY_END]. Firmware update code already have an
appropriate check to skip those areas when validating updated
firmware. Firmware programming code, OTOH, does not and will
needlessly send no-op I2C traffic. Add an appropriate check to
__ziirave_firm_write_pkt() so as to save all of that wasted effort.

While at it, normalize all of the address handling code to use full
32-bit address in units of bytes and convert it to an appropriate
value only in places where that is necessary.

Signed-off-by: Andrey Smirnov 
Cc: Chris Healy 
Cc: Guenter Roeck 
Cc: Rick Ramstetter 
Cc: linux-watch...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/watchdog/ziirave_wdt.c | 33 -
 1 file changed, 24 insertions(+), 9 deletions(-)

diff --git a/drivers/watchdog/ziirave_wdt.c b/drivers/watchdog/ziirave_wdt.c
index f05095b08016..a3cc936858ad 100644
--- a/drivers/watchdog/ziirave_wdt.c
+++ b/drivers/watchdog/ziirave_wdt.c
@@ -51,8 +51,8 @@ static char *ziirave_reasons[] = {"power cycle", "hw 
watchdog", NULL, NULL,
 
 #define ZIIRAVE_FIRM_PKT_TOTAL_SIZE20
 #define ZIIRAVE_FIRM_PKT_DATA_SIZE 16
-#define ZIIRAVE_FIRM_FLASH_MEMORY_START0x1600
-#define ZIIRAVE_FIRM_FLASH_MEMORY_END  0x2bbf
+#define ZIIRAVE_FIRM_FLASH_MEMORY_START(2 * 0x1600)
+#define ZIIRAVE_FIRM_FLASH_MEMORY_END  (2 * 0x2bbf)
 #define ZIIRAVE_FIRM_PAGE_SIZE 128
 
 /* Received and ready for next Download packet. */
@@ -195,12 +195,13 @@ static int ziirave_firm_wait_for_ack(struct 
watchdog_device *wdd)
return ret == ZIIRAVE_FIRM_DOWNLOAD_ACK ? 0 : -EIO;
 }
 
-static int ziirave_firm_set_read_addr(struct watchdog_device *wdd, u16 addr)
+static int ziirave_firm_set_read_addr(struct watchdog_device *wdd, u32 addr)
 {
struct i2c_client *client = to_i2c_client(wdd->parent);
+   const u16 addr16 = (u16)addr / 2;
u8 address[2];
 
-   put_unaligned_le16(addr, address);
+   put_unaligned_le16(addr16, address);
 
return i2c_smbus_write_block_data(client,
  ZIIRAVE_CMD_DOWNLOAD_SET_READ_ADDR,
@@ -234,6 +235,12 @@ static int ziirave_firm_write_byte(struct watchdog_device 
*wdd, u8 command,
 wait_for_ack);
 }
 
+static bool ziirave_firm_addr_readonly(u32 addr)
+{
+   return addr < ZIIRAVE_FIRM_FLASH_MEMORY_START ||
+  addr > ZIIRAVE_FIRM_FLASH_MEMORY_END;
+}
+
 /*
  * ziirave_firm_write_pkt() - Build and write a firmware packet
  *
@@ -261,6 +268,16 @@ static int __ziirave_firm_write_pkt(struct watchdog_device 
*wdd,
return -EMSGSIZE;
}
 
+   /*
+* Ignore packets that are targeting program memory outisde of
+* app partition, since they will be ignored by the
+* bootloader. At the same time, we need to make sure we'll
+* allow zero length packet that will be sent as the last step
+* of firmware update
+*/
+   if (len && ziirave_firm_addr_readonly(addr))
+   return 0;
+
/* Packet length */
packet[0] = len;
/* Packet address */
@@ -279,7 +296,7 @@ static int __ziirave_firm_write_pkt(struct watchdog_device 
*wdd,
if (ret)
dev_err(>dev,
  "Failed to write firmware packet at address 0x%04x: %d\n",
- addr16, ret);
+ addr, ret);
 
return ret;
 }
@@ -315,14 +332,12 @@ static int ziirave_firm_verify(struct watchdog_device 
*wdd,
const struct ihex_binrec *rec;
int i, ret;
u8 data[ZIIRAVE_FIRM_PKT_DATA_SIZE];
-   u16 addr;
 
for (rec = (void *)fw->data; rec; rec = ihex_next_binrec(rec)) {
const u16 len = be16_to_cpu(rec->len);
+   const u32 addr = be32_to_cpu(rec->addr);
 
-   addr = (be32_to_cpu(rec->addr) & 0x) >> 1;
-   if (addr < ZIIRAVE_FIRM_FLASH_MEMORY_START ||
-   addr > ZIIRAVE_FIRM_FLASH_MEMORY_END)
+   if (ziirave_firm_addr_readonly(addr))
continue;
 
ret = ziirave_firm_set_read_addr(wdd, addr);
-- 
2.21.0