[PATCH 0/9] SDM630/36/60 driver enablement

[PATCH 0/9] SDM630/36/60 driver enablement

[Konrad Dybcio]

Hi!

This series brings most of the changes that have been
brought to sdm630/636/660 SoCs apart from device tree
part (I cannot add anything unless some [1] smmu quirks
are merged.. blame qcom!)

So far, the platform is shaping up nicely, though some
peripherals seem to be living their own lives (I'm looking
at you, modem!)

The QPNP-REVID driver is taken from downstream but seems to
work fine upstream. I understand it may need some cleaning-up
before it's merged.

Current out-of-tree progress can be seen at [2].

[1] https://patchwork.kernel.org/patch/11643603/
[2] https://github.com/konradybcio/linux/commits/ninges_labs

Konrad Dybcio (8):
  clk: qcom: gcc-sdm660: Add missing modem reset
  phy: qcom-qusb2: Add support for SDM630/660
  drivers: usb: dwc3-qcom: Add sdm660 compatible
  drm/msm/dsi: Add phy configuration for SDM630/636/660
  drm/msm/mdp5: Add MDP5 configuration for SDM630
  drm/msm/dsi: Add DSI configuration for SDM660
  drm/msm/mdp5: Add MDP5 configuration for SDM636/660
  clk: qcom: gcc-sdm660: Fix up gcc_mss_mnoc_bimc_axi_clk

Xiaozhe Shi (1):
  soc/qcom: Add REVID driver

 .../devicetree/bindings/display/msm/dsi.txt   |   1 +
 .../bindings/phy/qcom,qusb2-phy.yaml          |   1 +
 .../bindings/soc/qcom/qcom,qpnp-revid.yaml    |  38 ++
 .../devicetree/bindings/usb/qcom,dwc3.yaml    |   1 +
 drivers/clk/qcom/gcc-sdm660.c                 |   4 +
 drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c      | 198 ++++++++++
 drivers/gpu/drm/msm/dsi/dsi_cfg.c             |  21 +
 drivers/gpu/drm/msm/dsi/dsi_cfg.h             |   1 +
 drivers/gpu/drm/msm/dsi/phy/dsi_phy.c         |   2 +
 drivers/gpu/drm/msm/dsi/phy/dsi_phy.h         |   1 +
 drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c    |  18 +
 drivers/phy/qualcomm/phy-qcom-qusb2.c         |   7 +-
 drivers/soc/qcom/Kconfig                      |   9 +
 drivers/soc/qcom/Makefile                     |   1 +
 drivers/soc/qcom/qpnp-revid.c                 | 288 ++++++++++++++
 drivers/usb/dwc3/dwc3-qcom.c                  |   1 +
 include/dt-bindings/clock/qcom,gcc-sdm660.h   |   1 +
 include/linux/qpnp/qpnp-revid.h               | 369 ++++++++++++++++++
 18 files changed, 961 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,qpnp-revid.yaml
 create mode 100644 drivers/soc/qcom/qpnp-revid.c
 create mode 100644 include/linux/qpnp/qpnp-revid.h

-- 
2.27.0

[PATCH 1/9] clk: qcom: gcc-sdm660: Add missing modem reset

[Konrad Dybcio]

This will be required in order to support the
modem upstream.

Signed-off-by: Konrad Dybcio <konradybcio@gmail.com>
---
 drivers/clk/qcom/gcc-sdm660.c               | 1 +
 include/dt-bindings/clock/qcom,gcc-sdm660.h | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/clk/qcom/gcc-sdm660.c b/drivers/clk/qcom/gcc-sdm660.c
index bf5730832ef3..a85283786278 100644
--- a/drivers/clk/qcom/gcc-sdm660.c
+++ b/drivers/clk/qcom/gcc-sdm660.c
@@ -2402,6 +2402,7 @@ static const struct qcom_reset_map gcc_sdm660_resets[] = {
 	[GCC_USB_20_BCR] = { 0x2f000 },
 	[GCC_USB_30_BCR] = { 0xf000 },
 	[GCC_USB_PHY_CFG_AHB2PHY_BCR] = { 0x6a000 },
+	[GCC_MSS_RESTART] = { 0x79000 },
 };
 
 static const struct regmap_config gcc_sdm660_regmap_config = {
diff --git a/include/dt-bindings/clock/qcom,gcc-sdm660.h b/include/dt-bindings/clock/qcom,gcc-sdm660.h
index 468302282913..df8a6f3d367e 100644
--- a/include/dt-bindings/clock/qcom,gcc-sdm660.h
+++ b/include/dt-bindings/clock/qcom,gcc-sdm660.h
@@ -152,5 +152,6 @@
 #define GCC_USB_20_BCR                  6
 #define GCC_USB_30_BCR			7
 #define GCC_USB_PHY_CFG_AHB2PHY_BCR	8
+#define GCC_MSS_RESTART			9
 
 #endif
-- 
2.27.0

Re: [PATCH 1/9] clk: qcom: gcc-sdm660: Add missing modem reset

[Konrad Dybcio]

Fixes: f2a76a2955c0 (clk: qcom: Add Global Clock controller (GCC)
driver for SDM660)

[PATCH 2/9] phy: qcom-qusb2: Add support for SDM630/660

[Konrad Dybcio]

QUSB on these SoCs actually uses *almost* the same
configuration that msm8996 does, so we can reuse
the phy_cfg from there with just a single change
(se clock scheme).

Signed-off-by: Konrad Dybcio <konradybcio@gmail.com>
---
 Documentation/devicetree/bindings/phy/qcom,qusb2-phy.yaml | 1 +
 drivers/phy/qualcomm/phy-qcom-qusb2.c                     | 7 ++++++-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/phy/qcom,qusb2-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,qusb2-phy.yaml
index b5a6195de7ff..e61a3ca3deba 100644
--- a/Documentation/devicetree/bindings/phy/qcom,qusb2-phy.yaml
+++ b/Documentation/devicetree/bindings/phy/qcom,qusb2-phy.yaml
@@ -20,6 +20,7 @@ properties:
         - enum:
           - qcom,msm8996-qusb2-phy
           - qcom,msm8998-qusb2-phy
+          - qcom,sdm660-qusb2-phy
       - items:
         - enum:
           - qcom,sc7180-qusb2-phy
diff --git a/drivers/phy/qualcomm/phy-qcom-qusb2.c b/drivers/phy/qualcomm/phy-qcom-qusb2.c
index 393011a05b48..11852ebe9e49 100644
--- a/drivers/phy/qualcomm/phy-qcom-qusb2.c
+++ b/drivers/phy/qualcomm/phy-qcom-qusb2.c
@@ -702,7 +702,8 @@ static int qusb2_phy_init(struct phy *phy)
 	usleep_range(150, 160);
 
 	/* Default is single-ended clock on msm8996 */
-	qphy->has_se_clk_scheme = true;
+	if (!of_device_is_compatible(phy->dev.of_node, "qcom,sdm660-qusb2-phy"))
+		qphy->has_se_clk_scheme = true;
 	/*
 	 * read TCSR_PHY_CLK_SCHEME register to check if single-ended
 	 * clock scheme is selected. If yes, then disable differential
@@ -815,6 +816,10 @@ static const struct of_device_id qusb2_phy_of_match_table[] = {
 	}, {
 		.compatible	= "qcom,msm8998-qusb2-phy",
 		.data		= &msm8998_phy_cfg,
+	}, {
+		.compatible	= "qcom,sdm660-qusb2-phy",
+		/* sdm630/660 use the same config as msm8996. */
+		.data		= &msm8996_phy_cfg,
 	}, {
 		/*
 		 * Deprecated. Only here to support legacy device
-- 
2.27.0

Re: [PATCH 2/9] phy: qcom-qusb2: Add support for SDM630/660

[Rob Herring]

On Sun, 26 Jul 2020 13:11:59 +0200, Konrad Dybcio wrote:
> QUSB on these SoCs actually uses *almost* the same
> configuration that msm8996 does, so we can reuse
> the phy_cfg from there with just a single change
> (se clock scheme).
> 
> Signed-off-by: Konrad Dybcio <konradybcio@gmail.com>
> ---
>  Documentation/devicetree/bindings/phy/qcom,qusb2-phy.yaml | 1 +
>  drivers/phy/qualcomm/phy-qcom-qusb2.c                     | 7 ++++++-
>  2 files changed, 7 insertions(+), 1 deletion(-)
> 

Acked-by: Rob Herring <robh@kernel.org>

[PATCH 3/9] drivers: usb: dwc3-qcom: Add sdm660 compatible

[Konrad Dybcio]

Signed-off-by: Konrad Dybcio <konradybcio@gmail.com>
---
 Documentation/devicetree/bindings/usb/qcom,dwc3.yaml | 1 +
 drivers/usb/dwc3/dwc3-qcom.c                         | 1 +
 2 files changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml b/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
index dac10848dd7f..c5280d6406ec 100644
--- a/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
+++ b/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
@@ -16,6 +16,7 @@ properties:
           - qcom,msm8996-dwc3
           - qcom,msm8998-dwc3
           - qcom,sc7180-dwc3
+          - qcom,sdm660-dwc3
           - qcom,sdm845-dwc3
       - const: qcom,dwc3
 
diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
index 1dfd024cd06b..d6b3e1bb550a 100644
--- a/drivers/usb/dwc3/dwc3-qcom.c
+++ b/drivers/usb/dwc3/dwc3-qcom.c
@@ -753,6 +753,7 @@ static const struct of_device_id dwc3_qcom_of_match[] = {
 	{ .compatible = "qcom,dwc3" },
 	{ .compatible = "qcom,msm8996-dwc3" },
 	{ .compatible = "qcom,msm8998-dwc3" },
+	{ .compatible = "qcom,sdm660-dwc3" },
 	{ .compatible = "qcom,sdm845-dwc3" },
 	{ }
 };
-- 
2.27.0

Re: [PATCH 3/9] drivers: usb: dwc3-qcom: Add sdm660 compatible

[Rob Herring]

On Sun, 26 Jul 2020 13:12:00 +0200, Konrad Dybcio wrote:
> Signed-off-by: Konrad Dybcio <konradybcio@gmail.com>
> ---
>  Documentation/devicetree/bindings/usb/qcom,dwc3.yaml | 1 +
>  drivers/usb/dwc3/dwc3-qcom.c                         | 1 +
>  2 files changed, 2 insertions(+)
> 

Acked-by: Rob Herring <robh@kernel.org>

[PATCH 4/9] drm/msm/dsi: Add phy configuration for SDM630/636/660

[Konrad Dybcio]

These SoCs make use of the 14nm phy, but at different
addresses than other 14nm units.

Signed-off-by: Konrad Dybcio <konradybcio@gmail.com>
---
 .../devicetree/bindings/display/msm/dsi.txt    |  1 +
 drivers/gpu/drm/msm/dsi/phy/dsi_phy.c          |  2 ++
 drivers/gpu/drm/msm/dsi/phy/dsi_phy.h          |  1 +
 drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c     | 18 ++++++++++++++++++
 4 files changed, 22 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/msm/dsi.txt b/Documentation/devicetree/bindings/display/msm/dsi.txt
index af95586c898f..7884fd7a85c1 100644
--- a/Documentation/devicetree/bindings/display/msm/dsi.txt
+++ b/Documentation/devicetree/bindings/display/msm/dsi.txt
@@ -87,6 +87,7 @@ Required properties:
   * "qcom,dsi-phy-20nm"
   * "qcom,dsi-phy-28nm-8960"
   * "qcom,dsi-phy-14nm"
+  * "qcom,dsi-phy-14nm-660"
   * "qcom,dsi-phy-10nm"
   * "qcom,dsi-phy-10nm-8998"
 - reg: Physical base address and length of the registers of PLL, PHY. Some
diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
index f509ebd77500..009f5b843dd1 100644
--- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
+++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
@@ -499,6 +499,8 @@ static const struct of_device_id dsi_phy_dt_match[] = {
 #ifdef CONFIG_DRM_MSM_DSI_14NM_PHY
 	{ .compatible = "qcom,dsi-phy-14nm",
 	  .data = &dsi_phy_14nm_cfgs },
+	{ .compatible = "qcom,dsi-phy-14nm-660",
+	  .data = &dsi_phy_14nm_660_cfgs },
 #endif
 #ifdef CONFIG_DRM_MSM_DSI_10NM_PHY
 	{ .compatible = "qcom,dsi-phy-10nm",
diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
index 24b294ed3059..ef8672d7b123 100644
--- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
+++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
@@ -45,6 +45,7 @@ extern const struct msm_dsi_phy_cfg dsi_phy_28nm_lp_cfgs;
 extern const struct msm_dsi_phy_cfg dsi_phy_20nm_cfgs;
 extern const struct msm_dsi_phy_cfg dsi_phy_28nm_8960_cfgs;
 extern const struct msm_dsi_phy_cfg dsi_phy_14nm_cfgs;
+extern const struct msm_dsi_phy_cfg dsi_phy_14nm_660_cfgs;
 extern const struct msm_dsi_phy_cfg dsi_phy_10nm_cfgs;
 extern const struct msm_dsi_phy_cfg dsi_phy_10nm_8998_cfgs;
 
diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c
index 1594f1422372..519400501bcd 100644
--- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c
+++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c
@@ -161,3 +161,21 @@ const struct msm_dsi_phy_cfg dsi_phy_14nm_cfgs = {
 	.io_start = { 0x994400, 0x996400 },
 	.num_dsi_phy = 2,
 };
+
+const struct msm_dsi_phy_cfg dsi_phy_14nm_660_cfgs = {
+	.type = MSM_DSI_PHY_14NM,
+	.src_pll_truthtable = { {false, false}, {true, false} },
+	.reg_cfg = {
+		.num = 1,
+		.regs = {
+			{"vcca", 17000, 32},
+		},
+	},
+	.ops = {
+		.enable = dsi_14nm_phy_enable,
+		.disable = dsi_14nm_phy_disable,
+		.init = dsi_14nm_phy_init,
+	},
+	.io_start = { 0xc994400, 0xc996000 },
+	.num_dsi_phy = 2,
+};
-- 
2.27.0

Re: [PATCH 4/9] drm/msm/dsi: Add phy configuration for SDM630/636/660

[Vinod Koul]

On 26-07-20, 13:12, Konrad Dybcio wrote:
> These SoCs make use of the 14nm phy, but at different
> addresses than other 14nm units.
> 
> Signed-off-by: Konrad Dybcio <konradybcio@gmail.com>
> ---
>  .../devicetree/bindings/display/msm/dsi.txt    |  1 +
>  drivers/gpu/drm/msm/dsi/phy/dsi_phy.c          |  2 ++
>  drivers/gpu/drm/msm/dsi/phy/dsi_phy.h          |  1 +
>  drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c     | 18 ++++++++++++++++++

Is there a reason why dsi phy needs to be here and not in phy subsystem
drivers/phy/ ?

>  4 files changed, 22 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/display/msm/dsi.txt b/Documentation/devicetree/bindings/display/msm/dsi.txt
> index af95586c898f..7884fd7a85c1 100644
> --- a/Documentation/devicetree/bindings/display/msm/dsi.txt
> +++ b/Documentation/devicetree/bindings/display/msm/dsi.txt
> @@ -87,6 +87,7 @@ Required properties:
>    * "qcom,dsi-phy-20nm"
>    * "qcom,dsi-phy-28nm-8960"
>    * "qcom,dsi-phy-14nm"
> +  * "qcom,dsi-phy-14nm-660"
>    * "qcom,dsi-phy-10nm"
>    * "qcom,dsi-phy-10nm-8998"
>  - reg: Physical base address and length of the registers of PLL, PHY. Some
> diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
> index f509ebd77500..009f5b843dd1 100644
> --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
> +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
> @@ -499,6 +499,8 @@ static const struct of_device_id dsi_phy_dt_match[] = {
>  #ifdef CONFIG_DRM_MSM_DSI_14NM_PHY
>  	{ .compatible = "qcom,dsi-phy-14nm",
>  	  .data = &dsi_phy_14nm_cfgs },
> +	{ .compatible = "qcom,dsi-phy-14nm-660",
> +	  .data = &dsi_phy_14nm_660_cfgs },
>  #endif
>  #ifdef CONFIG_DRM_MSM_DSI_10NM_PHY
>  	{ .compatible = "qcom,dsi-phy-10nm",
> diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
> index 24b294ed3059..ef8672d7b123 100644
> --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
> +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
> @@ -45,6 +45,7 @@ extern const struct msm_dsi_phy_cfg dsi_phy_28nm_lp_cfgs;
>  extern const struct msm_dsi_phy_cfg dsi_phy_20nm_cfgs;
>  extern const struct msm_dsi_phy_cfg dsi_phy_28nm_8960_cfgs;
>  extern const struct msm_dsi_phy_cfg dsi_phy_14nm_cfgs;
> +extern const struct msm_dsi_phy_cfg dsi_phy_14nm_660_cfgs;
>  extern const struct msm_dsi_phy_cfg dsi_phy_10nm_cfgs;
>  extern const struct msm_dsi_phy_cfg dsi_phy_10nm_8998_cfgs;
>  
> diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c
> index 1594f1422372..519400501bcd 100644
> --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c
> +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c
> @@ -161,3 +161,21 @@ const struct msm_dsi_phy_cfg dsi_phy_14nm_cfgs = {
>  	.io_start = { 0x994400, 0x996400 },
>  	.num_dsi_phy = 2,
>  };
> +
> +const struct msm_dsi_phy_cfg dsi_phy_14nm_660_cfgs = {
> +	.type = MSM_DSI_PHY_14NM,
> +	.src_pll_truthtable = { {false, false}, {true, false} },
> +	.reg_cfg = {
> +		.num = 1,
> +		.regs = {
> +			{"vcca", 17000, 32},
> +		},
> +	},
> +	.ops = {
> +		.enable = dsi_14nm_phy_enable,
> +		.disable = dsi_14nm_phy_disable,
> +		.init = dsi_14nm_phy_init,
> +	},
> +	.io_start = { 0xc994400, 0xc996000 },
> +	.num_dsi_phy = 2,
> +};
> -- 
> 2.27.0

-- 
~Vinod

Re: [PATCH 4/9] drm/msm/dsi: Add phy configuration for SDM630/636/660

[Rob Clark]

On Mon, Aug 3, 2020 at 4:00 AM Vinod Koul <vkoul@kernel.org> wrote:
>
> On 26-07-20, 13:12, Konrad Dybcio wrote:
> > These SoCs make use of the 14nm phy, but at different
> > addresses than other 14nm units.
> >
> > Signed-off-by: Konrad Dybcio <konradybcio@gmail.com>
> > ---
> >  .../devicetree/bindings/display/msm/dsi.txt    |  1 +
> >  drivers/gpu/drm/msm/dsi/phy/dsi_phy.c          |  2 ++
> >  drivers/gpu/drm/msm/dsi/phy/dsi_phy.h          |  1 +
> >  drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c     | 18 ++++++++++++++++++
>
> Is there a reason why dsi phy needs to be here and not in phy subsystem
> drivers/phy/ ?

*maybe* it would be possible to split out all of the dsi (and hdmi)
phy to drivers/phy.  But splitting out just the new ones wouldn't be
practical (it would duplicate a lot of code, and make the rest of the
dsi code have to deal with both cases).  And unlike dp/usb-c I'm not
really sure I see an advantage to justify the churn.

BR,
-R

>
> >  4 files changed, 22 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/display/msm/dsi.txt b/Documentation/devicetree/bindings/display/msm/dsi.txt
> > index af95586c898f..7884fd7a85c1 100644
> > --- a/Documentation/devicetree/bindings/display/msm/dsi.txt
> > +++ b/Documentation/devicetree/bindings/display/msm/dsi.txt
> > @@ -87,6 +87,7 @@ Required properties:
> >    * "qcom,dsi-phy-20nm"
> >    * "qcom,dsi-phy-28nm-8960"
> >    * "qcom,dsi-phy-14nm"
> > +  * "qcom,dsi-phy-14nm-660"
> >    * "qcom,dsi-phy-10nm"
> >    * "qcom,dsi-phy-10nm-8998"
> >  - reg: Physical base address and length of the registers of PLL, PHY. Some
> > diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
> > index f509ebd77500..009f5b843dd1 100644
> > --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
> > +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
> > @@ -499,6 +499,8 @@ static const struct of_device_id dsi_phy_dt_match[] = {
> >  #ifdef CONFIG_DRM_MSM_DSI_14NM_PHY
> >       { .compatible = "qcom,dsi-phy-14nm",
> >         .data = &dsi_phy_14nm_cfgs },
> > +     { .compatible = "qcom,dsi-phy-14nm-660",
> > +       .data = &dsi_phy_14nm_660_cfgs },
> >  #endif
> >  #ifdef CONFIG_DRM_MSM_DSI_10NM_PHY
> >       { .compatible = "qcom,dsi-phy-10nm",
> > diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
> > index 24b294ed3059..ef8672d7b123 100644
> > --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
> > +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
> > @@ -45,6 +45,7 @@ extern const struct msm_dsi_phy_cfg dsi_phy_28nm_lp_cfgs;
> >  extern const struct msm_dsi_phy_cfg dsi_phy_20nm_cfgs;
> >  extern const struct msm_dsi_phy_cfg dsi_phy_28nm_8960_cfgs;
> >  extern const struct msm_dsi_phy_cfg dsi_phy_14nm_cfgs;
> > +extern const struct msm_dsi_phy_cfg dsi_phy_14nm_660_cfgs;
> >  extern const struct msm_dsi_phy_cfg dsi_phy_10nm_cfgs;
> >  extern const struct msm_dsi_phy_cfg dsi_phy_10nm_8998_cfgs;
> >
> > diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c
> > index 1594f1422372..519400501bcd 100644
> > --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c
> > +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c
> > @@ -161,3 +161,21 @@ const struct msm_dsi_phy_cfg dsi_phy_14nm_cfgs = {
> >       .io_start = { 0x994400, 0x996400 },
> >       .num_dsi_phy = 2,
> >  };
> > +
> > +const struct msm_dsi_phy_cfg dsi_phy_14nm_660_cfgs = {
> > +     .type = MSM_DSI_PHY_14NM,
> > +     .src_pll_truthtable = { {false, false}, {true, false} },
> > +     .reg_cfg = {
> > +             .num = 1,
> > +             .regs = {
> > +                     {"vcca", 17000, 32},
> > +             },
> > +     },
> > +     .ops = {
> > +             .enable = dsi_14nm_phy_enable,
> > +             .disable = dsi_14nm_phy_disable,
> > +             .init = dsi_14nm_phy_init,
> > +     },
> > +     .io_start = { 0xc994400, 0xc996000 },
> > +     .num_dsi_phy = 2,
> > +};
> > --
> > 2.27.0
>
> --
> ~Vinod

Re: [PATCH 4/9] drm/msm/dsi: Add phy configuration for SDM630/636/660

[Vinod Koul]

On 03-08-20, 09:06, Rob Clark wrote:
> On Mon, Aug 3, 2020 at 4:00 AM Vinod Koul <vkoul@kernel.org> wrote:
> >
> > On 26-07-20, 13:12, Konrad Dybcio wrote:
> > > These SoCs make use of the 14nm phy, but at different
> > > addresses than other 14nm units.
> > >
> > > Signed-off-by: Konrad Dybcio <konradybcio@gmail.com>
> > > ---
> > >  .../devicetree/bindings/display/msm/dsi.txt    |  1 +
> > >  drivers/gpu/drm/msm/dsi/phy/dsi_phy.c          |  2 ++
> > >  drivers/gpu/drm/msm/dsi/phy/dsi_phy.h          |  1 +
> > >  drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c     | 18 ++++++++++++++++++
> >
> > Is there a reason why dsi phy needs to be here and not in phy subsystem
> > drivers/phy/ ?
> 
> *maybe* it would be possible to split out all of the dsi (and hdmi)
> phy to drivers/phy.  But splitting out just the new ones wouldn't be
> practical (it would duplicate a lot of code, and make the rest of the
> dsi code have to deal with both cases).  And unlike dp/usb-c I'm not
> really sure I see an advantage to justify the churn.

So the question would be if it helps in reuse if we do that and does it
result in a better solution than dsi code managing the phy. The
advantage of framework (like phy) is that different subsystems can use
a (phy) driver and common framework helps reduce duplicates.

Yes sure the question was not for a new phy but about the whole
msm/dsi/phy code and future for it.

-- 
~Vinod

Re: [PATCH 4/9] drm/msm/dsi: Add phy configuration for SDM630/636/660

[Rob Clark]

On Tue, Aug 4, 2020 at 5:09 AM Vinod Koul <vkoul@kernel.org> wrote:
>
> On 03-08-20, 09:06, Rob Clark wrote:
> > On Mon, Aug 3, 2020 at 4:00 AM Vinod Koul <vkoul@kernel.org> wrote:
> > >
> > > On 26-07-20, 13:12, Konrad Dybcio wrote:
> > > > These SoCs make use of the 14nm phy, but at different
> > > > addresses than other 14nm units.
> > > >
> > > > Signed-off-by: Konrad Dybcio <konradybcio@gmail.com>
> > > > ---
> > > >  .../devicetree/bindings/display/msm/dsi.txt    |  1 +
> > > >  drivers/gpu/drm/msm/dsi/phy/dsi_phy.c          |  2 ++
> > > >  drivers/gpu/drm/msm/dsi/phy/dsi_phy.h          |  1 +
> > > >  drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c     | 18 ++++++++++++++++++
> > >
> > > Is there a reason why dsi phy needs to be here and not in phy subsystem
> > > drivers/phy/ ?
> >
> > *maybe* it would be possible to split out all of the dsi (and hdmi)
> > phy to drivers/phy.  But splitting out just the new ones wouldn't be
> > practical (it would duplicate a lot of code, and make the rest of the
> > dsi code have to deal with both cases).  And unlike dp/usb-c I'm not
> > really sure I see an advantage to justify the churn.
>
> So the question would be if it helps in reuse if we do that and does it
> result in a better solution than dsi code managing the phy. The
> advantage of framework (like phy) is that different subsystems can use
> a (phy) driver and common framework helps reduce duplicates.

I'm not aware of any re-use that would be possible by splitting it
out.. if there were, it would be a more compelling argument.

It does increase the complexity and possibilities for getting kernel
config wrong.  There are devices like the aarch64 laptops which do not
have a debug serial port, where debugging issues like that can be a
pain when you get no display.  OTOH that might be balanced out a bit
by using a common framework/api that others are familiar with.

Overall, nowhere near high enough on my priority list to spend time
on.. there are bigger fires.  If someone was really motivated about
this and wanted to send (tested) patches, then I'd take a look and see
how it turned out.

BR,
-R

> Yes sure the question was not for a new phy but about the whole
> msm/dsi/phy code and future for it.
>
> --
> ~Vinod

[PATCH 5/9] drm/msm/mdp5: Add MDP5 configuration for SDM630

[Konrad Dybcio]

This commit adds support for the MDP5 IP on Snapdragon
630. The configuration is different from SDM660's, as
the latter one has two DSI outputs.

Signed-off-by: Konrad Dybcio <konradybcio@gmail.com>
---
 drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c | 93 ++++++++++++++++++++++++
 1 file changed, 93 insertions(+)

diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c
index 25a13a2a57a9..2e02de8a7e41 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c
@@ -910,6 +910,98 @@ static const struct mdp5_cfg_hw msm8998_config = {
 	.max_clk = 412500000,
 };
 
+static const struct mdp5_cfg_hw sdm630_config = {
+	.name = "sdm630",
+	.mdp = {
+		.count = 1,
+		.caps = MDP_CAP_CDM |
+			MDP_CAP_SRC_SPLIT |
+			0,
+	},
+	.ctl = {
+		.count = 5,
+		.base = { 0x01000, 0x01200, 0x01400, 0x01600, 0x01800 },
+		.flush_hw_mask = 0xf4ffffff,
+	},
+	.pipe_vig = {
+		.count = 1,
+		.base = { 0x04000 },
+		.caps = MDP_PIPE_CAP_HFLIP	|
+			MDP_PIPE_CAP_VFLIP	|
+			MDP_PIPE_CAP_SCALE	|
+			MDP_PIPE_CAP_CSC	|
+			MDP_PIPE_CAP_DECIMATION	|
+			MDP_PIPE_CAP_SW_PIX_EXT	|
+			0,
+	},
+	.pipe_rgb = {
+		.count = 4,
+		.base = { 0x14000, 0x16000, 0x18000, 0x1a000 },
+		.caps = MDP_PIPE_CAP_HFLIP	|
+			MDP_PIPE_CAP_VFLIP	|
+			MDP_PIPE_CAP_SCALE	|
+			MDP_PIPE_CAP_DECIMATION	|
+			MDP_PIPE_CAP_SW_PIX_EXT	|
+			0,
+	},
+	.pipe_dma = {
+		.count = 2, /* driver supports max of 2 currently */
+		.base = { 0x24000, 0x26000, 0x28000 },
+		.caps = MDP_PIPE_CAP_HFLIP	|
+			MDP_PIPE_CAP_VFLIP	|
+			MDP_PIPE_CAP_SW_PIX_EXT	|
+			0,
+	},
+	.pipe_cursor = {
+		.count = 1,
+		.base = { 0x34000 },
+		.caps = MDP_PIPE_CAP_HFLIP	|
+			MDP_PIPE_CAP_VFLIP	|
+			MDP_PIPE_CAP_SW_PIX_EXT	|
+			MDP_PIPE_CAP_CURSOR	|
+			0,
+	},
+
+	.lm = {
+		.count = 2,
+		.base = { 0x44000, 0x46000 },
+		.instances = {
+				{ .id = 0, .pp = 0, .dspp = 0,
+				  .caps = MDP_LM_CAP_DISPLAY |
+					  MDP_LM_CAP_PAIR, },
+				{ .id = 1, .pp = 1, .dspp = -1,
+				  .caps = MDP_LM_CAP_WB, },
+				},
+		.nb_stages = 8,
+		.max_width = 2048,
+		.max_height = 0xFFFF,
+	},
+	.dspp = {
+		.count = 1,
+		.base = { 0x54000 },
+	},
+	.ad = {
+		.count = 2,
+		.base = { 0x78000, 0x78800 },
+	},
+	.pp = {
+		.count = 3,
+		.base = { 0x70000, 0x71000, 0x72000 },
+	},
+	.cdm = {
+		.count = 1,
+		.base = { 0x79200 },
+	},
+	.intf = {
+		.base = { 0x6a000, 0x6a800 },
+		.connect = {
+			[0] = INTF_DISABLED,
+			[1] = INTF_DSI,
+		},
+	},
+	.max_clk = 412500000,
+};
+
 static const struct mdp5_cfg_handler cfg_handlers_v1[] = {
 	{ .revision = 0, .config = { .hw = &msm8x74v1_config } },
 	{ .revision = 2, .config = { .hw = &msm8x74v2_config } },
@@ -924,6 +1016,7 @@ static const struct mdp5_cfg_handler cfg_handlers_v1[] = {
 
 static const struct mdp5_cfg_handler cfg_handlers_v3[] = {
 	{ .revision = 0, .config = { .hw = &msm8998_config } },
+	{ .revision = 3, .config = { .hw = &sdm630_config } },
 };
 
 static struct mdp5_cfg_platform *mdp5_get_config(struct platform_device *dev);
-- 
2.27.0

[PATCH 6/9] drm/msm/dsi: Add DSI configuration for SDM660

[Konrad Dybcio]

This also applies to sdm630/636 and their SDA
counterparts.

Signed-off-by: Konrad Dybcio <konradybcio@gmail.com>
---
 drivers/gpu/drm/msm/dsi/dsi_cfg.c | 21 +++++++++++++++++++++
 drivers/gpu/drm/msm/dsi/dsi_cfg.h |  1 +
 2 files changed, 22 insertions(+)

diff --git a/drivers/gpu/drm/msm/dsi/dsi_cfg.c b/drivers/gpu/drm/msm/dsi/dsi_cfg.c
index 813d69deb5e8..f892f2cbe8bb 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_cfg.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_cfg.c
@@ -149,6 +149,25 @@ static const struct msm_dsi_config msm8998_dsi_cfg = {
 	.num_dsi = 2,
 };
 
+static const char * const dsi_sdm660_bus_clk_names[] = {
+	"iface", "bus", "core", "core_mmss",
+};
+
+static const struct msm_dsi_config sdm660_dsi_cfg = {
+	.io_offset = DSI_6G_REG_SHIFT,
+	.reg_cfg = {
+		.num = 2,
+		.regs = {
+			{"vdd", 73400, 32 },	/* 0.9 V */
+			{"vdda", 12560, 4 },	/* 1.2 V */
+		},
+	},
+	.bus_clk_names = dsi_sdm660_bus_clk_names,
+	.num_bus_clks = ARRAY_SIZE(dsi_sdm660_bus_clk_names),
+	.io_start = { 0xc994000, 0xc996000 },
+	.num_dsi = 2,
+};
+
 static const char * const dsi_sdm845_bus_clk_names[] = {
 	"iface", "bus",
 };
@@ -240,6 +259,8 @@ static const struct msm_dsi_cfg_handler dsi_cfg_handlers[] = {
 		&msm8996_dsi_cfg, &msm_dsi_6g_host_ops},
 	{MSM_DSI_VER_MAJOR_6G, MSM_DSI_6G_VER_MINOR_V1_4_2,
 		&msm8976_dsi_cfg, &msm_dsi_6g_host_ops},
+	{MSM_DSI_VER_MAJOR_6G, MSM_DSI_6G_VER_MINOR_V2_1_0,
+		&sdm660_dsi_cfg, &msm_dsi_6g_v2_host_ops},
 	{MSM_DSI_VER_MAJOR_6G, MSM_DSI_6G_VER_MINOR_V2_2_0,
 		&msm8998_dsi_cfg, &msm_dsi_6g_v2_host_ops},
 	{MSM_DSI_VER_MAJOR_6G, MSM_DSI_6G_VER_MINOR_V2_2_1,
diff --git a/drivers/gpu/drm/msm/dsi/dsi_cfg.h b/drivers/gpu/drm/msm/dsi/dsi_cfg.h
index 217e24a65178..efd469d1db45 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_cfg.h
+++ b/drivers/gpu/drm/msm/dsi/dsi_cfg.h
@@ -18,6 +18,7 @@
 #define MSM_DSI_6G_VER_MINOR_V1_3_1	0x10030001
 #define MSM_DSI_6G_VER_MINOR_V1_4_1	0x10040001
 #define MSM_DSI_6G_VER_MINOR_V1_4_2	0x10040002
+#define MSM_DSI_6G_VER_MINOR_V2_1_0	0x20010000
 #define MSM_DSI_6G_VER_MINOR_V2_2_0	0x20000000
 #define MSM_DSI_6G_VER_MINOR_V2_2_1	0x20020001
 #define MSM_DSI_6G_VER_MINOR_V2_4_1	0x20040001
-- 
2.27.0

[PATCH 7/9] drm/msm/mdp5: Add MDP5 configuration for SDM636/660

[Konrad Dybcio]

This commit adds support for the MDP5 IP on Snapdragon
636/660.

Signed-off-by: Konrad Dybcio <konradybcio@gmail.com>
---
 drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c | 105 +++++++++++++++++++++++
 1 file changed, 105 insertions(+)

diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c
index 2e02de8a7e41..df10c1ac7591 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c
@@ -1002,6 +1002,110 @@ static const struct mdp5_cfg_hw sdm630_config = {
 	.max_clk = 412500000,
 };
 
+static const struct mdp5_cfg_hw sdm660_config = {
+	.name = "sdm660",
+	.mdp = {
+		.count = 1,
+		.caps = MDP_CAP_DSC |
+			MDP_CAP_CDM |
+			MDP_CAP_SRC_SPLIT |
+			0,
+	},
+	.ctl = {
+		.count = 5,
+		.base = { 0x01000, 0x01200, 0x01400, 0x01600, 0x01800 },
+		.flush_hw_mask = 0xf4ffffff,
+	},
+	.pipe_vig = {
+		.count = 2,
+		.base = { 0x04000, 0x6000 },
+		.caps = MDP_PIPE_CAP_HFLIP	|
+			MDP_PIPE_CAP_VFLIP	|
+			MDP_PIPE_CAP_SCALE	|
+			MDP_PIPE_CAP_CSC	|
+			MDP_PIPE_CAP_DECIMATION	|
+			MDP_PIPE_CAP_SW_PIX_EXT	|
+			0,
+	},
+	.pipe_rgb = {
+		.count = 4,
+		.base = { 0x14000, 0x16000, 0x18000, 0x1a000 },
+		.caps = MDP_PIPE_CAP_HFLIP	|
+			MDP_PIPE_CAP_VFLIP	|
+			MDP_PIPE_CAP_SCALE	|
+			MDP_PIPE_CAP_DECIMATION	|
+			MDP_PIPE_CAP_SW_PIX_EXT	|
+			0,
+	},
+	.pipe_dma = {
+		.count = 2, /* driver supports max of 2 currently */
+		.base = { 0x24000, 0x26000, 0x28000 },
+		.caps = MDP_PIPE_CAP_HFLIP	|
+			MDP_PIPE_CAP_VFLIP	|
+			MDP_PIPE_CAP_SW_PIX_EXT	|
+			0,
+	},
+	.pipe_cursor = {
+		.count = 1,
+		.base = { 0x34000 },
+		.caps = MDP_PIPE_CAP_HFLIP	|
+			MDP_PIPE_CAP_VFLIP	|
+			MDP_PIPE_CAP_SW_PIX_EXT	|
+			MDP_PIPE_CAP_CURSOR	|
+			0,
+	},
+
+	.lm = {
+		.count = 4,
+		.base = { 0x44000, 0x45000, 0x46000, 0x49000 },
+		.instances = {
+				{ .id = 0, .pp = 0, .dspp = 0,
+				  .caps = MDP_LM_CAP_DISPLAY |
+					  MDP_LM_CAP_PAIR, },
+				{ .id = 1, .pp = 1, .dspp = 1,
+				  .caps = MDP_LM_CAP_DISPLAY, },
+				{ .id = 2, .pp = 2, .dspp = -1,
+				  .caps = MDP_LM_CAP_DISPLAY |
+					  MDP_LM_CAP_PAIR, },
+				{ .id = 3, .pp = 3, .dspp = -1,
+				  .caps = MDP_LM_CAP_WB, },
+				},
+		.nb_stages = 8,
+		.max_width = 2560,
+		.max_height = 0xFFFF,
+	},
+	.dspp = {
+		.count = 2,
+		.base = { 0x54000, 0x56000 },
+	},
+	.ad = {
+		.count = 2,
+		.base = { 0x78000, 0x78800 },
+	},
+	.pp = {
+		.count = 5,
+		.base = { 0x70000, 0x70800, 0x71000, 0x71800, 0x72000 },
+	},
+	.cdm = {
+		.count = 1,
+		.base = { 0x79200 },
+	},
+	.dsc = {
+		.count = 2,
+		.base = { 0x80000, 0x80400 },
+	},
+	.intf = {
+		.base = { 0x6a000, 0x6a800, 0x6b000, 0x6b800 },
+		.connect = {
+			[0] = INTF_DISABLED,
+			[1] = INTF_DSI,
+			[2] = INTF_DSI,
+			[3] = INTF_HDMI,
+		},
+	},
+	.max_clk = 412500000,
+};
+
 static const struct mdp5_cfg_handler cfg_handlers_v1[] = {
 	{ .revision = 0, .config = { .hw = &msm8x74v1_config } },
 	{ .revision = 2, .config = { .hw = &msm8x74v2_config } },
@@ -1016,6 +1120,7 @@ static const struct mdp5_cfg_handler cfg_handlers_v1[] = {
 
 static const struct mdp5_cfg_handler cfg_handlers_v3[] = {
 	{ .revision = 0, .config = { .hw = &msm8998_config } },
+	{ .revision = 2, .config = { .hw = &sdm660_config } },
 	{ .revision = 3, .config = { .hw = &sdm630_config } },
 };
 
-- 
2.27.0

[PATCH 8/9] clk: qcom: gcc-sdm660: Fix up gcc_mss_mnoc_bimc_axi_clk

[Konrad Dybcio]

Add missing halt_check, hwcg_reg and hwcg_bit properties.
These were likely omitted when porting the driver upstream.

Signed-off-by: Konrad Dybcio <konradybcio@gmail.com>
---
 drivers/clk/qcom/gcc-sdm660.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/clk/qcom/gcc-sdm660.c b/drivers/clk/qcom/gcc-sdm660.c
index a85283786278..f0b47b7d50ca 100644
--- a/drivers/clk/qcom/gcc-sdm660.c
+++ b/drivers/clk/qcom/gcc-sdm660.c
@@ -1715,6 +1715,9 @@ static struct clk_branch gcc_mss_cfg_ahb_clk = {
 
 static struct clk_branch gcc_mss_mnoc_bimc_axi_clk = {
 	.halt_reg = 0x8a004,
+	.halt_check = BRANCH_HALT,
+	.hwcg_reg = 0x8a004,
+	.hwcg_bit = 1,
 	.clkr = {
 		.enable_reg = 0x8a004,
 		.enable_mask = BIT(0),
-- 
2.27.0

Re: [PATCH 8/9] clk: qcom: gcc-sdm660: Fix up gcc_mss_mnoc_bimc_axi_clk

[Konrad Dybcio]

>Fixes tag?

Can I add it here? I supose I can.

Fixes: f2a76a2955c0 (clk: qcom: Add Global Clock controller (GCC)
driver for SDM660)

[PATCH 9/9] soc/qcom: Add REVID driver

[Konrad Dybcio]

From: Xiaozhe Shi <xiaozhes@codeaurora.org>

Add the REVID device driver. The REVID driver will print out the PMIC
revision at probe time.

Signed-off-by: Xiaozhe Shi <xiaozhes@codeaurora.org>
[konradybcio@gmail.com: Fast-forward the driver from kernel 4.14 to 5.8,
convert binding to yaml]
Signed-off-by: Konrad Dybcio <konradybcio@gmail.com>
---
 .../bindings/soc/qcom/qcom,qpnp-revid.yaml    |  38 ++
 drivers/soc/qcom/Kconfig                      |   9 +
 drivers/soc/qcom/Makefile                     |   1 +
 drivers/soc/qcom/qpnp-revid.c                 | 288 ++++++++++++++
 include/linux/qpnp/qpnp-revid.h               | 369 ++++++++++++++++++
 5 files changed, 705 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,qpnp-revid.yaml
 create mode 100644 drivers/soc/qcom/qpnp-revid.c
 create mode 100644 include/linux/qpnp/qpnp-revid.h

diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,qpnp-revid.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,qpnp-revid.yaml
new file mode 100644
index 000000000000..fa4a15d7cc15
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/qcom/qcom,qpnp-revid.yaml
@@ -0,0 +1,38 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/soc/qcom/qcom,qpnp-revid.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: QPNP-REVID
+
+description: |
+ QPNP-REVID provides a way to read the PMIC part number and revision.
+
+properties:
+  compatible:
+    enum:
+      - qcom,qpnp-revid
+
+  reg:
+    description: Offset and length of the PMIC peripheral register map.
+    maxItems: 1
+
+  qcom,fab-id-valid:
+    description: |
+     Use this property when support to read Fab
+	   identification from REV ID peripheral is available.
+
+  qcom,tp-rev-valid:
+    description: |
+     Use this property when support to read TP
+	   revision identification from REV ID peripheral.
+
+examples:
+  - |
+    qcom,revid@100 {
+        compatible = "qcom,qpnp-revid";
+        reg = <0x100 0x100>;
+    };
+
+...
diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
index 07bb261a63d2..03a024acdb37 100644
--- a/drivers/soc/qcom/Kconfig
+++ b/drivers/soc/qcom/Kconfig
@@ -75,6 +75,15 @@ config QCOM_QMI_HELPERS
 	tristate
 	depends on NET
 
+config QCOM_QPNP_REVID
+	tristate "QPNP Revision ID Peripheral"
+	depends on SPMI
+	help
+	  Say 'y' here to include support for the Qualcomm Technologies, Inc.
+	  QPNP REVID peripheral. REVID prints out the PMIC type and revision
+	  numbers in the kernel log along with the PMIC option status. The PMIC
+	  type is mapped to a QTI chip part number and logged as well.
+
 config QCOM_RMTFS_MEM
 	tristate "Qualcomm Remote Filesystem memory driver"
 	depends on ARCH_QCOM
diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
index 7d7e2ecbdce6..d7a4c6b85ede 100644
--- a/drivers/soc/qcom/Makefile
+++ b/drivers/soc/qcom/Makefile
@@ -9,6 +9,7 @@ obj-$(CONFIG_QCOM_OCMEM)	+= ocmem.o
 obj-$(CONFIG_QCOM_PDR_HELPERS)	+= pdr_interface.o
 obj-$(CONFIG_QCOM_QMI_HELPERS)	+= qmi_helpers.o
 qmi_helpers-y	+= qmi_encdec.o qmi_interface.o
+obj-$(CONFIG_QCOM_QPNP_REVID)	+= qpnp-revid.o
 obj-$(CONFIG_QCOM_RMTFS_MEM)	+= rmtfs_mem.o
 obj-$(CONFIG_QCOM_RPMH)		+= qcom_rpmh.o
 qcom_rpmh-y			+= rpmh-rsc.o
diff --git a/drivers/soc/qcom/qpnp-revid.c b/drivers/soc/qcom/qpnp-revid.c
new file mode 100644
index 000000000000..edae0b299847
--- /dev/null
+++ b/drivers/soc/qcom/qpnp-revid.c
@@ -0,0 +1,288 @@
+/* Copyright (c) 2013-2019, The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/spmi.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/err.h>
+#include <linux/qpnp/qpnp-revid.h>
+#include <linux/of.h>
+
+#define REVID_REVISION1	0x0
+#define REVID_REVISION2	0x1
+#define REVID_REVISION3	0x2
+#define REVID_REVISION4	0x3
+#define REVID_TYPE	0x4
+#define REVID_SUBTYPE	0x5
+#define REVID_STATUS1	0x8
+#define REVID_SPARE_0	0x60
+#define REVID_TP_REV	0xf1
+#define REVID_FAB_ID	0xf2
+
+#define QPNP_REVID_DEV_NAME "qcom,qpnp-revid"
+
+static const char *const pmic_names[] = {
+	[0] =	"Unknown PMIC",
+	[PM8941_SUBTYPE] = "PM8941",
+	[PM8841_SUBTYPE] = "PM8841",
+	[PM8019_SUBTYPE] = "PM8019",
+	[PM8226_SUBTYPE] = "PM8226",
+	[PM8110_SUBTYPE] = "PM8110",
+	[PMA8084_SUBTYPE] = "PMA8084",
+	[PMI8962_SUBTYPE] = "PMI8962",
+	[PMD9635_SUBTYPE] = "PMD9635",
+	[PM8994_SUBTYPE] = "PM8994",
+	[PMI8994_SUBTYPE] = "PMI8994",
+	[PM8916_SUBTYPE] = "PM8916",
+	[PM8004_SUBTYPE] = "PM8004",
+	[PM8909_SUBTYPE] = "PM8909",
+	[PM2433_SUBTYPE] = "PM2433",
+	[PMD9655_SUBTYPE] = "PMD9655",
+	[PM8950_SUBTYPE] = "PM8950",
+	[PMI8950_SUBTYPE] = "PMI8950",
+	[PMK8001_SUBTYPE] = "PMK8001",
+	[PMI8996_SUBTYPE] = "PMI8996",
+	[PM8998_SUBTYPE] = "PM8998",
+	[PMI8998_SUBTYPE] = "PMI8998",
+	[PM8005_SUBTYPE] = "PM8005",
+	[PM8937_SUBTYPE] = "PM8937",
+	[PM660L_SUBTYPE] = "PM660L",
+	[PM660_SUBTYPE] = "PM660",
+	[PMI632_SUBTYPE] = "PMI632",
+	[PMI8937_SUBTYPE] = "PMI8937",
+	[PM8150_SUBTYPE] = "PM8150",
+	[PM8150B_SUBTYPE] = "PM8150B",
+	[PM8150L_SUBTYPE] = "PM8150L",
+	[PM6150_SUBTYPE] = "PM6150",
+	[PM8150A_SUBTYPE] = "PM8150A",
+	[PME9205_SUBTYPE] = "PME9205",
+	[PM6125_SUBTYPE] = "PM6125",
+	[PM8008_SUBTYPE] = "PM8008",
+	[SMB1355_SUBTYPE] = "SMB1355",
+	[SMB1390_SUBTYPE] = "SMB1390",
+};
+
+struct revid_chip {
+	struct list_head	link;
+	struct device_node	*dev_node;
+	struct pmic_revid_data	data;
+};
+
+static LIST_HEAD(revid_chips);
+static DEFINE_MUTEX(revid_chips_lock);
+
+static const struct of_device_id qpnp_revid_match_table[] = {
+	{ .compatible = QPNP_REVID_DEV_NAME },
+	{}
+};
+
+static u8 qpnp_read_byte(struct regmap *regmap, u16 addr)
+{
+	int rc;
+	int val;
+
+	rc = regmap_read(regmap, addr, &val);
+	if (rc) {
+		pr_err("read failed rc=%d\n", rc);
+		return 0;
+	}
+	return (u8)val;
+}
+
+/**
+ * get_revid_data - Return the revision information of PMIC
+ * @dev_node: Pointer to the revid peripheral of the PMIC for which
+ *		revision information is seeked
+ *
+ * CONTEXT: Should be called in non atomic context
+ *
+ * RETURNS: pointer to struct pmic_revid_data filled with the information
+ *		about the PMIC revision
+ */
+struct pmic_revid_data *get_revid_data(struct device_node *dev_node)
+{
+	struct revid_chip *revid_chip;
+
+	if (!dev_node)
+		return ERR_PTR(-EINVAL);
+
+	mutex_lock(&revid_chips_lock);
+	list_for_each_entry(revid_chip, &revid_chips, link) {
+		if (dev_node == revid_chip->dev_node) {
+			mutex_unlock(&revid_chips_lock);
+			return &revid_chip->data;
+		}
+	}
+	mutex_unlock(&revid_chips_lock);
+	return ERR_PTR(-EINVAL);
+}
+EXPORT_SYMBOL(get_revid_data);
+
+#define PM8941_PERIPHERAL_SUBTYPE	0x01
+#define PM8226_PERIPHERAL_SUBTYPE	0x04
+#define PMD9655_PERIPHERAL_SUBTYPE	0x0F
+#define PMI8950_PERIPHERAL_SUBTYPE	0x11
+#define PMI8937_PERIPHERAL_SUBTYPE	0x37
+static size_t build_pmic_string(char *buf, size_t n, int sid,
+		u8 subtype, u8 rev1, u8 rev2, u8 rev3, u8 rev4)
+{
+	size_t pos = 0;
+	/*
+	 * In early versions of PM8941 and PM8226, the major revision number
+	 * started incrementing from 0 (eg 0 = v1.0, 1 = v2.0).
+	 * Increment the major revision number here if the chip is an early
+	 * version of PM8941 or PM8226.
+	 */
+	if (((int)subtype == PM8941_PERIPHERAL_SUBTYPE
+			|| (int)subtype == PM8226_PERIPHERAL_SUBTYPE)
+			&& rev4 < 0x02)
+		rev4++;
+
+	pos += snprintf(buf + pos, n - pos, "PMIC@SID%d", sid);
+	if (subtype >= ARRAY_SIZE(pmic_names) || subtype == 0)
+		pos += snprintf(buf + pos, n - pos, ": %s (subtype: 0x%02X)",
+				pmic_names[0], subtype);
+	else
+		pos += snprintf(buf + pos, n - pos, ": %s",
+				pmic_names[subtype]);
+	pos += snprintf(buf + pos, n - pos, " v%d.%d", rev4, rev3);
+	if (rev2 || rev1)
+		pos += snprintf(buf + pos, n - pos, ".%d", rev2);
+	if (rev1)
+		pos += snprintf(buf + pos, n - pos, ".%d", rev1);
+	return pos;
+}
+
+#define PMIC_PERIPHERAL_TYPE		0x51
+#define PMIC_STRING_MAXLENGTH		80
+static int qpnp_revid_probe(struct platform_device *pdev)
+{
+	u8 rev1, rev2, rev3, rev4, pmic_type, pmic_subtype, pmic_status;
+	u8 option1, option2, option3, option4, spare0;
+	unsigned int base;
+	int rc, fab_id, tp_rev;
+	char pmic_string[PMIC_STRING_MAXLENGTH] = {'\0'};
+	struct revid_chip *revid_chip;
+	struct regmap *regmap;
+
+	regmap = dev_get_regmap(pdev->dev.parent, NULL);
+	if (!regmap) {
+		dev_err(&pdev->dev, "Couldn't get parent's regmap\n");
+		return -EINVAL;
+	}
+
+	rc = of_property_read_u32(pdev->dev.of_node, "reg", &base);
+	if (rc < 0) {
+		dev_err(&pdev->dev,
+			"Couldn't find reg in node = %s rc = %d\n",
+			pdev->dev.of_node->full_name, rc);
+		return rc;
+	}
+	pmic_type = qpnp_read_byte(regmap, base + REVID_TYPE);
+	if (pmic_type != PMIC_PERIPHERAL_TYPE) {
+		pr_err("Invalid REVID peripheral type: %02X\n", pmic_type);
+		return -EINVAL;
+	}
+
+	rev1 = qpnp_read_byte(regmap, base + REVID_REVISION1);
+	rev2 = qpnp_read_byte(regmap, base + REVID_REVISION2);
+	rev3 = qpnp_read_byte(regmap, base + REVID_REVISION3);
+	rev4 = qpnp_read_byte(regmap, base + REVID_REVISION4);
+
+	pmic_subtype = qpnp_read_byte(regmap, base + REVID_SUBTYPE);
+	if (pmic_subtype != PMD9655_PERIPHERAL_SUBTYPE)
+		pmic_status = qpnp_read_byte(regmap, base + REVID_STATUS1);
+	else
+		pmic_status = 0;
+
+	/* special case for PMI8937 */
+	if (pmic_subtype == PMI8950_PERIPHERAL_SUBTYPE) {
+		/* read spare register */
+		spare0 = qpnp_read_byte(regmap, base + REVID_SPARE_0);
+		if (spare0)
+			pmic_subtype = PMI8937_PERIPHERAL_SUBTYPE;
+	}
+
+	if (of_property_read_bool(pdev->dev.of_node, "qcom,fab-id-valid"))
+		fab_id = qpnp_read_byte(regmap, base + REVID_FAB_ID);
+	else
+		fab_id = -EINVAL;
+
+	if (of_property_read_bool(pdev->dev.of_node, "qcom,tp-rev-valid"))
+		tp_rev = qpnp_read_byte(regmap, base + REVID_TP_REV);
+	else
+		tp_rev = -EINVAL;
+
+	revid_chip = devm_kzalloc(&pdev->dev, sizeof(struct revid_chip),
+						GFP_KERNEL);
+	if (!revid_chip)
+		return -ENOMEM;
+
+	revid_chip->dev_node = pdev->dev.of_node;
+	revid_chip->data.rev1 = rev1;
+	revid_chip->data.rev2 = rev2;
+	revid_chip->data.rev3 = rev3;
+	revid_chip->data.rev4 = rev4;
+	revid_chip->data.pmic_subtype = pmic_subtype;
+	revid_chip->data.pmic_type = pmic_type;
+	revid_chip->data.fab_id = fab_id;
+	revid_chip->data.tp_rev = tp_rev;
+
+	if (pmic_subtype < ARRAY_SIZE(pmic_names))
+		revid_chip->data.pmic_name = pmic_names[pmic_subtype];
+	else
+		revid_chip->data.pmic_name = pmic_names[0];
+
+	mutex_lock(&revid_chips_lock);
+	list_add(&revid_chip->link, &revid_chips);
+	mutex_unlock(&revid_chips_lock);
+
+	option1 = pmic_status & 0x3;
+	option2 = (pmic_status >> 2) & 0x3;
+	option3 = (pmic_status >> 4) & 0x3;
+	option4 = (pmic_status >> 6) & 0x3;
+
+	build_pmic_string(pmic_string, PMIC_STRING_MAXLENGTH,
+			  to_spmi_device(pdev->dev.parent)->usid,
+			pmic_subtype, rev1, rev2, rev3, rev4);
+	pr_info("%s options: %d, %d, %d, %d\n",
+			pmic_string, option1, option2, option3, option4);
+	return 0;
+}
+
+static struct platform_driver qpnp_revid_driver = {
+	.probe	= qpnp_revid_probe,
+	.driver	= {
+		.name		= QPNP_REVID_DEV_NAME,
+		.owner		= THIS_MODULE,
+		.of_match_table	= qpnp_revid_match_table,
+	},
+};
+
+static int __init qpnp_revid_init(void)
+{
+	return platform_driver_register(&qpnp_revid_driver);
+}
+
+static void __exit qpnp_revid_exit(void)
+{
+	return platform_driver_unregister(&qpnp_revid_driver);
+}
+
+subsys_initcall(qpnp_revid_init);
+module_exit(qpnp_revid_exit);
+
+MODULE_DESCRIPTION("QPNP REVID DRIVER");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:" QPNP_REVID_DEV_NAME);
diff --git a/include/linux/qpnp/qpnp-revid.h b/include/linux/qpnp/qpnp-revid.h
new file mode 100644
index 000000000000..0fbdd528d204
--- /dev/null
+++ b/include/linux/qpnp/qpnp-revid.h
@@ -0,0 +1,369 @@
+/* Copyright (c) 2013-2019, The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef __QPNP_REVID
+#define __QPNP_REVID
+
+/* Common TYPE for all PMICs */
+#define PMIC_TYPE		0x51
+
+/* PM8994 */
+#define PM8941_SUBTYPE		0x01
+
+#define PM8941_V1P0_REV1	0x00
+#define PM8941_V1P0_REV2	0x00
+#define PM8941_V1P0_REV3	0x00
+#define PM8941_V1P0_REV4	0x01
+
+#define PM8941_V2P0_REV1	0x00
+#define PM8941_V2P0_REV2	0x00
+#define PM8941_V2P0_REV3	0x00
+#define PM8941_V2P0_REV4	0x01
+
+#define PM8941_V3P0_REV1	0x00
+#define PM8941_V3P0_REV2	0x00
+#define PM8941_V3P0_REV3	0x00
+#define PM8941_V3P0_REV4	0x03
+
+#define PM8941_V3P1_REV1	0x00
+#define PM8941_V3P1_REV2	0x00
+#define PM8941_V3P1_REV3	0x01
+#define PM8941_V3P1_REV4	0x03
+
+/* PM8841 */
+#define PM8841_SUBTYPE		0x02
+
+/* PM8019 */
+#define PM8019_SUBTYPE		0x03
+
+/* PM8226 */
+#define PM8226_SUBTYPE		0x04
+
+#define PM8226_V2P2_REV1	0x00
+#define PM8226_V2P2_REV2	0x00
+#define PM8226_V2P2_REV3	0x02
+#define PM8226_V2P2_REV4	0x02
+
+#define PM8226_V2P1_REV1	0x00
+#define PM8226_V2P1_REV2	0x00
+#define PM8226_V2P1_REV3	0x01
+#define PM8226_V2P1_REV4	0x02
+
+#define PM8226_V2P0_REV1	0x00
+#define PM8226_V2P0_REV2	0x00
+#define PM8226_V2P0_REV3	0x00
+#define PM8226_V2P0_REV4	0x02
+
+#define PM8226_V1P0_REV1	0x00
+#define PM8226_V1P0_REV2	0x00
+#define PM8226_V1P0_REV3	0x00
+#define PM8226_V1P0_REV4	0x00
+
+/* PM8110 */
+#define PM8110_SUBTYPE		0x05
+
+#define PM8110_V1P0_REV1	0x00
+#define PM8110_V1P0_REV2	0x00
+#define PM8110_V1P0_REV3	0x00
+#define PM8110_V1P0_REV4	0x01
+
+#define PM8110_V1P1_REV1	0x00
+#define PM8110_V1P1_REV2	0x01
+#define PM8110_V1P1_REV3	0x00
+#define PM8110_V1P1_REV4	0x01
+
+#define PM8110_V1P3_REV1	0x00
+#define PM8110_V1P3_REV2	0x03
+#define PM8110_V1P3_REV3	0x00
+#define PM8110_V1P3_REV4	0x01
+
+#define PM8110_V2P0_REV1	0x00
+#define PM8110_V2P0_REV2	0x00
+#define PM8110_V2P0_REV3	0x00
+#define PM8110_V2P0_REV4	0x02
+
+/* PMA8084 */
+#define PMA8084_SUBTYPE		0x06
+
+/* PMI8962 */
+#define PMI8962_SUBTYPE		0x07
+
+/* PMD9635 */
+#define PMD9635_SUBTYPE		0x08
+/* PM8994 */
+#define PM8994_SUBTYPE		0x09
+
+/* PMI8994 */
+#define PMI8994_TYPE		0x51
+#define PMI8994_SUBTYPE		0x0A
+
+#define PMI8994_V1P0_REV1	0x00
+#define PMI8994_V1P0_REV2	0x00
+#define PMI8994_V1P0_REV3	0x00
+#define PMI8994_V1P0_REV4	0x01
+
+#define PMI8994_V2P0_REV1	0x00
+#define PMI8994_V2P0_REV2	0x00
+#define PMI8994_V2P0_REV3	0x00
+#define PMI8994_V2P0_REV4	0x02
+
+/* PM8916 */
+#define PM8916_SUBTYPE		0x0B
+
+#define PM8916_V1P0_REV1	0x00
+#define PM8916_V1P0_REV2	0x00
+#define PM8916_V1P0_REV3	0x00
+#define PM8916_V1P0_REV4	0x01
+
+#define PM8916_V1P1_REV1	0x00
+#define PM8916_V1P1_REV2	0x00
+#define PM8916_V1P1_REV3	0x01
+#define PM8916_V1P1_REV4	0x01
+
+#define PM8916_V2P0_REV1	0x00
+#define PM8916_V2P0_REV2	0x00
+#define PM8916_V2P0_REV3	0x00
+#define PM8916_V2P0_REV4	0x02
+
+/* PM8004 */
+#define PM8004_SUBTYPE		0x0C
+
+/* PM8909 */
+#define PM8909_SUBTYPE		0x0D
+
+#define PM8909_V1P0_REV1	0x00
+#define PM8909_V1P0_REV2	0x00
+#define PM8909_V1P0_REV3	0x00
+#define PM8909_V1P0_REV4	0x01
+
+#define PM8909_V1P1_REV1	0x00
+#define PM8909_V1P1_REV2	0x00
+#define PM8909_V1P1_REV3	0x01
+#define PM8909_V1P1_REV4	0x01
+
+/* PM2433 */
+#define PM2433_SUBTYPE		0x0E
+
+/* PMD9655 */
+#define PMD9655_SUBTYPE		0x0F
+
+/* PM8950 */
+#define PM8950_SUBTYPE		0x10
+#define PM8950_V1P0_REV4	0x01
+
+#define PM8950_V2P0_REV4	0x02
+
+/* PMI8950 */
+#define PMI8950_SUBTYPE		0x11
+
+/* PMK8001 */
+#define PMK8001_SUBTYPE		0x12
+
+/* PMI8996 */
+#define PMI8996_SUBTYPE		0x13
+
+/* PM8998 */
+#define PM8998_SUBTYPE	0x14
+
+/* PMI8998 */
+#define PMI8998_SUBTYPE	0x15
+
+/* PM660 */
+#define PM660L_SUBTYPE	0x1A
+#define PM660_SUBTYPE	0x1B
+
+/* PM8150 */
+#define PM8150_SUBTYPE		0x1E
+#define PM8150L_SUBTYPE		0x1F
+#define PM8150B_SUBTYPE		0x20
+#define PM8150A_SUBTYPE		0x27
+
+/* PM6150 SUBTYPE */
+#define PM6150_SUBTYPE		0x28
+#define PM6150L_SUBTYPE		0x1F
+
+/* PME9205 SUBTYPE */
+#define PME9205_SUBTYPE		0x24
+
+/* PM6125 SUBTYPE */
+#define PM6125_SUBTYPE		0x2D
+
+/* PMI632 */
+#define PMI632_SUBTYPE	0x25
+
+/* PM8008 SUBTYPE */
+#define PM8008_SUBTYPE	0x2C
+
+/* PMI8998 REV_ID */
+#define PMI8998_V1P0_REV1	0x00
+#define PMI8998_V1P0_REV2	0x00
+#define PMI8998_V1P0_REV3	0x00
+#define PMI8998_V1P0_REV4	0x01
+
+#define PMI8998_V1P1_REV1	0x00
+#define PMI8998_V1P1_REV2	0x00
+#define PMI8998_V1P1_REV3	0x01
+#define PMI8998_V1P1_REV4	0x01
+
+#define PMI8998_V2P0_REV1	0x00
+#define PMI8998_V2P0_REV2	0x00
+#define PMI8998_V2P0_REV3	0x00
+#define PMI8998_V2P0_REV4	0x02
+
+/* PM660 REV_ID */
+#define PM660_V1P0_REV1		0x00
+#define PM660_V1P0_REV2		0x00
+#define PM660_V1P0_REV3		0x00
+#define PM660_V1P0_REV4		0x01
+
+#define PM660_V1P1_REV1		0x00
+#define PM660_V1P1_REV2		0x00
+#define PM660_V1P1_REV3		0x01
+#define PM660_V1P1_REV4		0x01
+
+/* PM660L REV_ID */
+#define PM660L_V1P1_REV1	0x00
+#define PM660L_V1P1_REV2	0x00
+#define PM660L_V1P1_REV3	0x01
+#define PM660L_V1P1_REV4	0x01
+
+#define PM660L_V2P0_REV1	0x00
+#define PM660L_V2P0_REV2	0x00
+#define PM660L_V2P0_REV3	0x00
+#define PM660L_V2P0_REV4	0x02
+
+/* PMI632 REV_ID */
+#define PMI632_V1P0_REV1	0x00
+#define PMI632_V1P0_REV2	0x00
+#define PMI632_V1P0_REV3	0x00
+#define PMI632_V1P0_REV4	0x01
+
+/* PM8150B_REV_ID */
+#define PM8150B_V1P0_REV1	0x00
+#define PM8150B_V1P0_REV2	0x00
+#define PM8150B_V1P0_REV3	0x00
+#define PM8150B_V1P0_REV4	0x01
+
+#define PM8150B_V2P0_REV1	0x00
+#define PM8150B_V2P0_REV2	0x00
+#define PM8150B_V2P0_REV3	0x00
+#define PM8150B_V2P0_REV4	0x02
+
+/* PM8150L_REV_ID */
+#define PM8150L_V1P0_REV1	0x00
+#define PM8150L_V1P0_REV2	0x00
+#define PM8150L_V1P0_REV3	0x00
+#define PM8150L_V1P0_REV4	0x01
+
+#define PM8150L_V2P0_REV1	0x00
+#define PM8150L_V2P0_REV2	0x00
+#define PM8150L_V2P0_REV3	0x00
+#define PM8150L_V2P0_REV4	0x02
+
+#define PM8150L_V3P0_REV1	0x00
+#define PM8150L_V3P0_REV2	0x00
+#define PM8150L_V3P0_REV3	0x00
+#define PM8150L_V3P0_REV4	0x03
+
+/* PM8150A_REV_ID */
+#define PM8150A_V1P0_REV1	0x00
+#define PM8150A_V1P0_REV2	0x00
+#define PM8150A_V1P0_REV3	0x00
+#define PM8150A_V1P0_REV4	0x01
+
+#define PM8150A_V2P0_REV1	0x00
+#define PM8150A_V2P0_REV2	0x00
+#define PM8150A_V2P0_REV3	0x00
+#define PM8150A_V2P0_REV4	0x02
+
+/* PME9205_REV_ID */
+#define PME9205_V1P0_REV1	0x00
+#define PME9205_V1P0_REV2	0x00
+#define PME9205_V1P0_REV3	0x00
+#define PME9205_V1P0_REV4	0x01
+
+#define PME9205_V2P0_REV1	0x00
+#define PME9205_V2P0_REV2	0x00
+#define PME9205_V2P0_REV3	0x00
+#define PME9205_V2P0_REV4	0x02
+
+/* PM6150_REV_ID */
+#define PM6150_V1P0_REV1	0x00
+#define PM6150_V1P0_REV2	0x00
+#define PM6150_V1P0_REV3	0x00
+#define PM6150_V1P0_REV4	0x01
+
+#define PM6150_V1P1_REV1	0x00
+#define PM6150_V1P1_REV2	0x00
+#define PM6150_V1P1_REV3	0x01
+#define PM6150_V1P1_REV4	0x01
+
+#define PM6150_V2P0_REV1	0x00
+#define PM6150_V2P0_REV2	0x00
+#define PM6150_V2P0_REV3	0x00
+#define PM6150_V2P0_REV4	0x02
+
+/* PM6125_REV_ID */
+#define PM6125_V1P0_REV1	0x00
+#define PM6125_V1P0_REV2	0x00
+#define PM6125_V1P0_REV3	0x00
+#define PM6125_V1P0_REV4	0x01
+
+/* PMI8998 FAB_ID */
+#define PMI8998_FAB_ID_SMIC	0x11
+#define PMI8998_FAB_ID_GF	0x30
+
+/* PM660 FAB_ID */
+#define PM660_FAB_ID_GF		0x0
+#define PM660_FAB_ID_TSMC	0x2
+#define PM660_FAB_ID_MX		0x3
+
+/* PM8005 */
+#define PM8005_SUBTYPE		0x18
+
+/* PM8937 */
+#define PM8937_SUBTYPE		0x19
+
+/* PMI8937 */
+#define PMI8937_SUBTYPE		0x37
+
+/* SMB1390 */
+#define SMB1390_SUBTYPE		0x23
+
+/* SMB1381 */
+#define SMB1381_SUBTYPE		0x17
+
+/* SMB1355 */
+#define SMB1355_SUBTYPE		0x1C
+
+struct pmic_revid_data {
+	u8		rev1;
+	u8		rev2;
+	u8		rev3;
+	u8		rev4;
+	u8		pmic_type;
+	u8		pmic_subtype;
+	const char	*pmic_name;
+	int		fab_id;
+	int		tp_rev;
+};
+
+#ifdef CONFIG_QCOM_QPNP_REVID
+struct pmic_revid_data *get_revid_data(struct device_node *dev_node);
+#else
+static inline
+struct pmic_revid_data *get_revid_data(struct device_node *dev_node)
+{
+	return NULL;
+}
+#endif
+#endif
-- 
2.27.0

Re: [PATCH 9/9] soc/qcom: Add REVID driver

[Greg Kroah-Hartman]

On Sun, Jul 26, 2020 at 01:12:06PM +0200, Konrad Dybcio wrote:
> From: Xiaozhe Shi <xiaozhes@codeaurora.org>
> 
> Add the REVID device driver. The REVID driver will print out the PMIC
> revision at probe time.

Why do we need this noise in the kernel log?

> --- /dev/null
> +++ b/drivers/soc/qcom/qpnp-revid.c
> @@ -0,0 +1,288 @@
> +/* Copyright (c) 2013-2019, The Linux Foundation. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 and
> + * only version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */

You can drop the GPL boilerplate text and add a proper SPDX line at the
top.

Didn't checkpatch ask for that?

> +
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <linux/spmi.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/err.h>
> +#include <linux/qpnp/qpnp-revid.h>
> +#include <linux/of.h>
> +
> +#define REVID_REVISION1	0x0
> +#define REVID_REVISION2	0x1
> +#define REVID_REVISION3	0x2
> +#define REVID_REVISION4	0x3
> +#define REVID_TYPE	0x4
> +#define REVID_SUBTYPE	0x5
> +#define REVID_STATUS1	0x8
> +#define REVID_SPARE_0	0x60
> +#define REVID_TP_REV	0xf1
> +#define REVID_FAB_ID	0xf2
> +
> +#define QPNP_REVID_DEV_NAME "qcom,qpnp-revid"
> +
> +static const char *const pmic_names[] = {
> +	[0] =	"Unknown PMIC",
> +	[PM8941_SUBTYPE] = "PM8941",
> +	[PM8841_SUBTYPE] = "PM8841",
> +	[PM8019_SUBTYPE] = "PM8019",
> +	[PM8226_SUBTYPE] = "PM8226",
> +	[PM8110_SUBTYPE] = "PM8110",
> +	[PMA8084_SUBTYPE] = "PMA8084",
> +	[PMI8962_SUBTYPE] = "PMI8962",
> +	[PMD9635_SUBTYPE] = "PMD9635",
> +	[PM8994_SUBTYPE] = "PM8994",
> +	[PMI8994_SUBTYPE] = "PMI8994",
> +	[PM8916_SUBTYPE] = "PM8916",
> +	[PM8004_SUBTYPE] = "PM8004",
> +	[PM8909_SUBTYPE] = "PM8909",
> +	[PM2433_SUBTYPE] = "PM2433",
> +	[PMD9655_SUBTYPE] = "PMD9655",
> +	[PM8950_SUBTYPE] = "PM8950",
> +	[PMI8950_SUBTYPE] = "PMI8950",
> +	[PMK8001_SUBTYPE] = "PMK8001",
> +	[PMI8996_SUBTYPE] = "PMI8996",
> +	[PM8998_SUBTYPE] = "PM8998",
> +	[PMI8998_SUBTYPE] = "PMI8998",
> +	[PM8005_SUBTYPE] = "PM8005",
> +	[PM8937_SUBTYPE] = "PM8937",
> +	[PM660L_SUBTYPE] = "PM660L",
> +	[PM660_SUBTYPE] = "PM660",
> +	[PMI632_SUBTYPE] = "PMI632",
> +	[PMI8937_SUBTYPE] = "PMI8937",
> +	[PM8150_SUBTYPE] = "PM8150",
> +	[PM8150B_SUBTYPE] = "PM8150B",
> +	[PM8150L_SUBTYPE] = "PM8150L",
> +	[PM6150_SUBTYPE] = "PM6150",
> +	[PM8150A_SUBTYPE] = "PM8150A",
> +	[PME9205_SUBTYPE] = "PME9205",
> +	[PM6125_SUBTYPE] = "PM6125",
> +	[PM8008_SUBTYPE] = "PM8008",
> +	[SMB1355_SUBTYPE] = "SMB1355",
> +	[SMB1390_SUBTYPE] = "SMB1390",
> +};
> +
> +struct revid_chip {
> +	struct list_head	link;
> +	struct device_node	*dev_node;
> +	struct pmic_revid_data	data;
> +};
> +
> +static LIST_HEAD(revid_chips);
> +static DEFINE_MUTEX(revid_chips_lock);
> +
> +static const struct of_device_id qpnp_revid_match_table[] = {
> +	{ .compatible = QPNP_REVID_DEV_NAME },
> +	{}
> +};
> +
> +static u8 qpnp_read_byte(struct regmap *regmap, u16 addr)
> +{
> +	int rc;
> +	int val;
> +
> +	rc = regmap_read(regmap, addr, &val);
> +	if (rc) {
> +		pr_err("read failed rc=%d\n", rc);

Drivers should always use dev_err() and friends, as you have access to a
struct device * always.  Please fix up the driver here to use that api
instead, no pr_* should be needed at all.

> +		return 0;
> +	}
> +	return (u8)val;
> +}
> +
> +/**
> + * get_revid_data - Return the revision information of PMIC
> + * @dev_node: Pointer to the revid peripheral of the PMIC for which
> + *		revision information is seeked
> + *
> + * CONTEXT: Should be called in non atomic context
> + *
> + * RETURNS: pointer to struct pmic_revid_data filled with the information
> + *		about the PMIC revision
> + */
> +struct pmic_revid_data *get_revid_data(struct device_node *dev_node)
> +{
> +	struct revid_chip *revid_chip;
> +
> +	if (!dev_node)
> +		return ERR_PTR(-EINVAL);
> +
> +	mutex_lock(&revid_chips_lock);
> +	list_for_each_entry(revid_chip, &revid_chips, link) {
> +		if (dev_node == revid_chip->dev_node) {
> +			mutex_unlock(&revid_chips_lock);
> +			return &revid_chip->data;
> +		}
> +	}
> +	mutex_unlock(&revid_chips_lock);
> +	return ERR_PTR(-EINVAL);
> +}
> +EXPORT_SYMBOL(get_revid_data);

Horrible global symbol name.  Who calls this?  This is the last patch in
the series, so if there is no user for this, please don't export it.

> +
> +#define PM8941_PERIPHERAL_SUBTYPE	0x01
> +#define PM8226_PERIPHERAL_SUBTYPE	0x04
> +#define PMD9655_PERIPHERAL_SUBTYPE	0x0F
> +#define PMI8950_PERIPHERAL_SUBTYPE	0x11
> +#define PMI8937_PERIPHERAL_SUBTYPE	0x37
> +static size_t build_pmic_string(char *buf, size_t n, int sid,
> +		u8 subtype, u8 rev1, u8 rev2, u8 rev3, u8 rev4)
> +{
> +	size_t pos = 0;
> +	/*
> +	 * In early versions of PM8941 and PM8226, the major revision number
> +	 * started incrementing from 0 (eg 0 = v1.0, 1 = v2.0).
> +	 * Increment the major revision number here if the chip is an early
> +	 * version of PM8941 or PM8226.
> +	 */
> +	if (((int)subtype == PM8941_PERIPHERAL_SUBTYPE
> +			|| (int)subtype == PM8226_PERIPHERAL_SUBTYPE)
> +			&& rev4 < 0x02)
> +		rev4++;
> +
> +	pos += snprintf(buf + pos, n - pos, "PMIC@SID%d", sid);
> +	if (subtype >= ARRAY_SIZE(pmic_names) || subtype == 0)
> +		pos += snprintf(buf + pos, n - pos, ": %s (subtype: 0x%02X)",
> +				pmic_names[0], subtype);
> +	else
> +		pos += snprintf(buf + pos, n - pos, ": %s",
> +				pmic_names[subtype]);
> +	pos += snprintf(buf + pos, n - pos, " v%d.%d", rev4, rev3);
> +	if (rev2 || rev1)
> +		pos += snprintf(buf + pos, n - pos, ".%d", rev2);
> +	if (rev1)
> +		pos += snprintf(buf + pos, n - pos, ".%d", rev1);
> +	return pos;
> +}
> +
> +#define PMIC_PERIPHERAL_TYPE		0x51
> +#define PMIC_STRING_MAXLENGTH		80
> +static int qpnp_revid_probe(struct platform_device *pdev)
> +{
> +	u8 rev1, rev2, rev3, rev4, pmic_type, pmic_subtype, pmic_status;
> +	u8 option1, option2, option3, option4, spare0;
> +	unsigned int base;
> +	int rc, fab_id, tp_rev;
> +	char pmic_string[PMIC_STRING_MAXLENGTH] = {'\0'};
> +	struct revid_chip *revid_chip;
> +	struct regmap *regmap;
> +
> +	regmap = dev_get_regmap(pdev->dev.parent, NULL);
> +	if (!regmap) {
> +		dev_err(&pdev->dev, "Couldn't get parent's regmap\n");
> +		return -EINVAL;
> +	}
> +
> +	rc = of_property_read_u32(pdev->dev.of_node, "reg", &base);
> +	if (rc < 0) {
> +		dev_err(&pdev->dev,
> +			"Couldn't find reg in node = %s rc = %d\n",
> +			pdev->dev.of_node->full_name, rc);
> +		return rc;
> +	}
> +	pmic_type = qpnp_read_byte(regmap, base + REVID_TYPE);
> +	if (pmic_type != PMIC_PERIPHERAL_TYPE) {
> +		pr_err("Invalid REVID peripheral type: %02X\n", pmic_type);
> +		return -EINVAL;
> +	}
> +
> +	rev1 = qpnp_read_byte(regmap, base + REVID_REVISION1);
> +	rev2 = qpnp_read_byte(regmap, base + REVID_REVISION2);
> +	rev3 = qpnp_read_byte(regmap, base + REVID_REVISION3);
> +	rev4 = qpnp_read_byte(regmap, base + REVID_REVISION4);
> +
> +	pmic_subtype = qpnp_read_byte(regmap, base + REVID_SUBTYPE);
> +	if (pmic_subtype != PMD9655_PERIPHERAL_SUBTYPE)
> +		pmic_status = qpnp_read_byte(regmap, base + REVID_STATUS1);
> +	else
> +		pmic_status = 0;
> +
> +	/* special case for PMI8937 */
> +	if (pmic_subtype == PMI8950_PERIPHERAL_SUBTYPE) {
> +		/* read spare register */
> +		spare0 = qpnp_read_byte(regmap, base + REVID_SPARE_0);
> +		if (spare0)
> +			pmic_subtype = PMI8937_PERIPHERAL_SUBTYPE;
> +	}
> +
> +	if (of_property_read_bool(pdev->dev.of_node, "qcom,fab-id-valid"))
> +		fab_id = qpnp_read_byte(regmap, base + REVID_FAB_ID);
> +	else
> +		fab_id = -EINVAL;
> +
> +	if (of_property_read_bool(pdev->dev.of_node, "qcom,tp-rev-valid"))
> +		tp_rev = qpnp_read_byte(regmap, base + REVID_TP_REV);
> +	else
> +		tp_rev = -EINVAL;
> +
> +	revid_chip = devm_kzalloc(&pdev->dev, sizeof(struct revid_chip),
> +						GFP_KERNEL);
> +	if (!revid_chip)
> +		return -ENOMEM;
> +
> +	revid_chip->dev_node = pdev->dev.of_node;
> +	revid_chip->data.rev1 = rev1;
> +	revid_chip->data.rev2 = rev2;
> +	revid_chip->data.rev3 = rev3;
> +	revid_chip->data.rev4 = rev4;
> +	revid_chip->data.pmic_subtype = pmic_subtype;
> +	revid_chip->data.pmic_type = pmic_type;
> +	revid_chip->data.fab_id = fab_id;
> +	revid_chip->data.tp_rev = tp_rev;
> +
> +	if (pmic_subtype < ARRAY_SIZE(pmic_names))
> +		revid_chip->data.pmic_name = pmic_names[pmic_subtype];
> +	else
> +		revid_chip->data.pmic_name = pmic_names[0];
> +
> +	mutex_lock(&revid_chips_lock);
> +	list_add(&revid_chip->link, &revid_chips);
> +	mutex_unlock(&revid_chips_lock);
> +
> +	option1 = pmic_status & 0x3;
> +	option2 = (pmic_status >> 2) & 0x3;
> +	option3 = (pmic_status >> 4) & 0x3;
> +	option4 = (pmic_status >> 6) & 0x3;
> +
> +	build_pmic_string(pmic_string, PMIC_STRING_MAXLENGTH,
> +			  to_spmi_device(pdev->dev.parent)->usid,
> +			pmic_subtype, rev1, rev2, rev3, rev4);
> +	pr_info("%s options: %d, %d, %d, %d\n",
> +			pmic_string, option1, option2, option3, option4);

Again, dev_info().

But really, why?  Who uses this information?  Drivers should be quiet if
all is working properly, no need to log anything.

> +	return 0;
> +}
> +
> +static struct platform_driver qpnp_revid_driver = {
> +	.probe	= qpnp_revid_probe,
> +	.driver	= {
> +		.name		= QPNP_REVID_DEV_NAME,
> +		.owner		= THIS_MODULE,
> +		.of_match_table	= qpnp_revid_match_table,
> +	},
> +};
> +
> +static int __init qpnp_revid_init(void)
> +{
> +	return platform_driver_register(&qpnp_revid_driver);
> +}
> +
> +static void __exit qpnp_revid_exit(void)
> +{
> +	return platform_driver_unregister(&qpnp_revid_driver);
> +}
> +
> +subsys_initcall(qpnp_revid_init);

Why subsys_initcall() if no one uses the function in this module?

Can't you just use the "normal" platform module init macros?

> +module_exit(qpnp_revid_exit);
> +
> +MODULE_DESCRIPTION("QPNP REVID DRIVER");
> +MODULE_LICENSE("GPL v2");
> +MODULE_ALIAS("platform:" QPNP_REVID_DEV_NAME);
> diff --git a/include/linux/qpnp/qpnp-revid.h b/include/linux/qpnp/qpnp-revid.h
> new file mode 100644
> index 000000000000..0fbdd528d204
> --- /dev/null
> +++ b/include/linux/qpnp/qpnp-revid.h

Why do you need a .h file in the include directory if only a single .c
file needs it?  Just put that info in the .c file itself.


> @@ -0,0 +1,369 @@
> +/* Copyright (c) 2013-2019, The Linux Foundation. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 and
> + * only version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#ifndef __QPNP_REVID
> +#define __QPNP_REVID
> +
> +/* Common TYPE for all PMICs */
> +#define PMIC_TYPE		0x51
> +
> +/* PM8994 */
> +#define PM8941_SUBTYPE		0x01
> +
> +#define PM8941_V1P0_REV1	0x00
> +#define PM8941_V1P0_REV2	0x00
> +#define PM8941_V1P0_REV3	0x00
> +#define PM8941_V1P0_REV4	0x01
> +
> +#define PM8941_V2P0_REV1	0x00
> +#define PM8941_V2P0_REV2	0x00
> +#define PM8941_V2P0_REV3	0x00
> +#define PM8941_V2P0_REV4	0x01
> +
> +#define PM8941_V3P0_REV1	0x00
> +#define PM8941_V3P0_REV2	0x00
> +#define PM8941_V3P0_REV3	0x00
> +#define PM8941_V3P0_REV4	0x03
> +
> +#define PM8941_V3P1_REV1	0x00
> +#define PM8941_V3P1_REV2	0x00
> +#define PM8941_V3P1_REV3	0x01
> +#define PM8941_V3P1_REV4	0x03
> +
> +/* PM8841 */
> +#define PM8841_SUBTYPE		0x02
> +
> +/* PM8019 */
> +#define PM8019_SUBTYPE		0x03
> +
> +/* PM8226 */
> +#define PM8226_SUBTYPE		0x04
> +
> +#define PM8226_V2P2_REV1	0x00
> +#define PM8226_V2P2_REV2	0x00
> +#define PM8226_V2P2_REV3	0x02
> +#define PM8226_V2P2_REV4	0x02
> +
> +#define PM8226_V2P1_REV1	0x00
> +#define PM8226_V2P1_REV2	0x00
> +#define PM8226_V2P1_REV3	0x01
> +#define PM8226_V2P1_REV4	0x02
> +
> +#define PM8226_V2P0_REV1	0x00
> +#define PM8226_V2P0_REV2	0x00
> +#define PM8226_V2P0_REV3	0x00
> +#define PM8226_V2P0_REV4	0x02
> +
> +#define PM8226_V1P0_REV1	0x00
> +#define PM8226_V1P0_REV2	0x00
> +#define PM8226_V1P0_REV3	0x00
> +#define PM8226_V1P0_REV4	0x00
> +
> +/* PM8110 */
> +#define PM8110_SUBTYPE		0x05
> +
> +#define PM8110_V1P0_REV1	0x00
> +#define PM8110_V1P0_REV2	0x00
> +#define PM8110_V1P0_REV3	0x00
> +#define PM8110_V1P0_REV4	0x01
> +
> +#define PM8110_V1P1_REV1	0x00
> +#define PM8110_V1P1_REV2	0x01
> +#define PM8110_V1P1_REV3	0x00
> +#define PM8110_V1P1_REV4	0x01
> +
> +#define PM8110_V1P3_REV1	0x00
> +#define PM8110_V1P3_REV2	0x03
> +#define PM8110_V1P3_REV3	0x00
> +#define PM8110_V1P3_REV4	0x01
> +
> +#define PM8110_V2P0_REV1	0x00
> +#define PM8110_V2P0_REV2	0x00
> +#define PM8110_V2P0_REV3	0x00
> +#define PM8110_V2P0_REV4	0x02
> +
> +/* PMA8084 */
> +#define PMA8084_SUBTYPE		0x06
> +
> +/* PMI8962 */
> +#define PMI8962_SUBTYPE		0x07
> +
> +/* PMD9635 */
> +#define PMD9635_SUBTYPE		0x08
> +/* PM8994 */
> +#define PM8994_SUBTYPE		0x09
> +
> +/* PMI8994 */
> +#define PMI8994_TYPE		0x51
> +#define PMI8994_SUBTYPE		0x0A
> +
> +#define PMI8994_V1P0_REV1	0x00
> +#define PMI8994_V1P0_REV2	0x00
> +#define PMI8994_V1P0_REV3	0x00
> +#define PMI8994_V1P0_REV4	0x01
> +
> +#define PMI8994_V2P0_REV1	0x00
> +#define PMI8994_V2P0_REV2	0x00
> +#define PMI8994_V2P0_REV3	0x00
> +#define PMI8994_V2P0_REV4	0x02
> +
> +/* PM8916 */
> +#define PM8916_SUBTYPE		0x0B
> +
> +#define PM8916_V1P0_REV1	0x00
> +#define PM8916_V1P0_REV2	0x00
> +#define PM8916_V1P0_REV3	0x00
> +#define PM8916_V1P0_REV4	0x01
> +
> +#define PM8916_V1P1_REV1	0x00
> +#define PM8916_V1P1_REV2	0x00
> +#define PM8916_V1P1_REV3	0x01
> +#define PM8916_V1P1_REV4	0x01
> +
> +#define PM8916_V2P0_REV1	0x00
> +#define PM8916_V2P0_REV2	0x00
> +#define PM8916_V2P0_REV3	0x00
> +#define PM8916_V2P0_REV4	0x02
> +
> +/* PM8004 */
> +#define PM8004_SUBTYPE		0x0C
> +
> +/* PM8909 */
> +#define PM8909_SUBTYPE		0x0D
> +
> +#define PM8909_V1P0_REV1	0x00
> +#define PM8909_V1P0_REV2	0x00
> +#define PM8909_V1P0_REV3	0x00
> +#define PM8909_V1P0_REV4	0x01
> +
> +#define PM8909_V1P1_REV1	0x00
> +#define PM8909_V1P1_REV2	0x00
> +#define PM8909_V1P1_REV3	0x01
> +#define PM8909_V1P1_REV4	0x01
> +
> +/* PM2433 */
> +#define PM2433_SUBTYPE		0x0E
> +
> +/* PMD9655 */
> +#define PMD9655_SUBTYPE		0x0F
> +
> +/* PM8950 */
> +#define PM8950_SUBTYPE		0x10
> +#define PM8950_V1P0_REV4	0x01
> +
> +#define PM8950_V2P0_REV4	0x02
> +
> +/* PMI8950 */
> +#define PMI8950_SUBTYPE		0x11
> +
> +/* PMK8001 */
> +#define PMK8001_SUBTYPE		0x12
> +
> +/* PMI8996 */
> +#define PMI8996_SUBTYPE		0x13
> +
> +/* PM8998 */
> +#define PM8998_SUBTYPE	0x14
> +
> +/* PMI8998 */
> +#define PMI8998_SUBTYPE	0x15
> +
> +/* PM660 */
> +#define PM660L_SUBTYPE	0x1A
> +#define PM660_SUBTYPE	0x1B
> +
> +/* PM8150 */
> +#define PM8150_SUBTYPE		0x1E
> +#define PM8150L_SUBTYPE		0x1F
> +#define PM8150B_SUBTYPE		0x20
> +#define PM8150A_SUBTYPE		0x27
> +
> +/* PM6150 SUBTYPE */
> +#define PM6150_SUBTYPE		0x28
> +#define PM6150L_SUBTYPE		0x1F
> +
> +/* PME9205 SUBTYPE */
> +#define PME9205_SUBTYPE		0x24
> +
> +/* PM6125 SUBTYPE */
> +#define PM6125_SUBTYPE		0x2D
> +
> +/* PMI632 */
> +#define PMI632_SUBTYPE	0x25
> +
> +/* PM8008 SUBTYPE */
> +#define PM8008_SUBTYPE	0x2C
> +
> +/* PMI8998 REV_ID */
> +#define PMI8998_V1P0_REV1	0x00
> +#define PMI8998_V1P0_REV2	0x00
> +#define PMI8998_V1P0_REV3	0x00
> +#define PMI8998_V1P0_REV4	0x01
> +
> +#define PMI8998_V1P1_REV1	0x00
> +#define PMI8998_V1P1_REV2	0x00
> +#define PMI8998_V1P1_REV3	0x01
> +#define PMI8998_V1P1_REV4	0x01
> +
> +#define PMI8998_V2P0_REV1	0x00
> +#define PMI8998_V2P0_REV2	0x00
> +#define PMI8998_V2P0_REV3	0x00
> +#define PMI8998_V2P0_REV4	0x02
> +
> +/* PM660 REV_ID */
> +#define PM660_V1P0_REV1		0x00
> +#define PM660_V1P0_REV2		0x00
> +#define PM660_V1P0_REV3		0x00
> +#define PM660_V1P0_REV4		0x01
> +
> +#define PM660_V1P1_REV1		0x00
> +#define PM660_V1P1_REV2		0x00
> +#define PM660_V1P1_REV3		0x01
> +#define PM660_V1P1_REV4		0x01
> +
> +/* PM660L REV_ID */
> +#define PM660L_V1P1_REV1	0x00
> +#define PM660L_V1P1_REV2	0x00
> +#define PM660L_V1P1_REV3	0x01
> +#define PM660L_V1P1_REV4	0x01
> +
> +#define PM660L_V2P0_REV1	0x00
> +#define PM660L_V2P0_REV2	0x00
> +#define PM660L_V2P0_REV3	0x00
> +#define PM660L_V2P0_REV4	0x02
> +
> +/* PMI632 REV_ID */
> +#define PMI632_V1P0_REV1	0x00
> +#define PMI632_V1P0_REV2	0x00
> +#define PMI632_V1P0_REV3	0x00
> +#define PMI632_V1P0_REV4	0x01
> +
> +/* PM8150B_REV_ID */
> +#define PM8150B_V1P0_REV1	0x00
> +#define PM8150B_V1P0_REV2	0x00
> +#define PM8150B_V1P0_REV3	0x00
> +#define PM8150B_V1P0_REV4	0x01
> +
> +#define PM8150B_V2P0_REV1	0x00
> +#define PM8150B_V2P0_REV2	0x00
> +#define PM8150B_V2P0_REV3	0x00
> +#define PM8150B_V2P0_REV4	0x02
> +
> +/* PM8150L_REV_ID */
> +#define PM8150L_V1P0_REV1	0x00
> +#define PM8150L_V1P0_REV2	0x00
> +#define PM8150L_V1P0_REV3	0x00
> +#define PM8150L_V1P0_REV4	0x01
> +
> +#define PM8150L_V2P0_REV1	0x00
> +#define PM8150L_V2P0_REV2	0x00
> +#define PM8150L_V2P0_REV3	0x00
> +#define PM8150L_V2P0_REV4	0x02
> +
> +#define PM8150L_V3P0_REV1	0x00
> +#define PM8150L_V3P0_REV2	0x00
> +#define PM8150L_V3P0_REV3	0x00
> +#define PM8150L_V3P0_REV4	0x03
> +
> +/* PM8150A_REV_ID */
> +#define PM8150A_V1P0_REV1	0x00
> +#define PM8150A_V1P0_REV2	0x00
> +#define PM8150A_V1P0_REV3	0x00
> +#define PM8150A_V1P0_REV4	0x01
> +
> +#define PM8150A_V2P0_REV1	0x00
> +#define PM8150A_V2P0_REV2	0x00
> +#define PM8150A_V2P0_REV3	0x00
> +#define PM8150A_V2P0_REV4	0x02
> +
> +/* PME9205_REV_ID */
> +#define PME9205_V1P0_REV1	0x00
> +#define PME9205_V1P0_REV2	0x00
> +#define PME9205_V1P0_REV3	0x00
> +#define PME9205_V1P0_REV4	0x01
> +
> +#define PME9205_V2P0_REV1	0x00
> +#define PME9205_V2P0_REV2	0x00
> +#define PME9205_V2P0_REV3	0x00
> +#define PME9205_V2P0_REV4	0x02
> +
> +/* PM6150_REV_ID */
> +#define PM6150_V1P0_REV1	0x00
> +#define PM6150_V1P0_REV2	0x00
> +#define PM6150_V1P0_REV3	0x00
> +#define PM6150_V1P0_REV4	0x01
> +
> +#define PM6150_V1P1_REV1	0x00
> +#define PM6150_V1P1_REV2	0x00
> +#define PM6150_V1P1_REV3	0x01
> +#define PM6150_V1P1_REV4	0x01
> +
> +#define PM6150_V2P0_REV1	0x00
> +#define PM6150_V2P0_REV2	0x00
> +#define PM6150_V2P0_REV3	0x00
> +#define PM6150_V2P0_REV4	0x02
> +
> +/* PM6125_REV_ID */
> +#define PM6125_V1P0_REV1	0x00
> +#define PM6125_V1P0_REV2	0x00
> +#define PM6125_V1P0_REV3	0x00
> +#define PM6125_V1P0_REV4	0x01
> +
> +/* PMI8998 FAB_ID */
> +#define PMI8998_FAB_ID_SMIC	0x11
> +#define PMI8998_FAB_ID_GF	0x30
> +
> +/* PM660 FAB_ID */
> +#define PM660_FAB_ID_GF		0x0
> +#define PM660_FAB_ID_TSMC	0x2
> +#define PM660_FAB_ID_MX		0x3
> +
> +/* PM8005 */
> +#define PM8005_SUBTYPE		0x18
> +
> +/* PM8937 */
> +#define PM8937_SUBTYPE		0x19
> +
> +/* PMI8937 */
> +#define PMI8937_SUBTYPE		0x37
> +
> +/* SMB1390 */
> +#define SMB1390_SUBTYPE		0x23
> +
> +/* SMB1381 */
> +#define SMB1381_SUBTYPE		0x17
> +
> +/* SMB1355 */
> +#define SMB1355_SUBTYPE		0x1C
> +
> +struct pmic_revid_data {
> +	u8		rev1;
> +	u8		rev2;
> +	u8		rev3;
> +	u8		rev4;
> +	u8		pmic_type;
> +	u8		pmic_subtype;
> +	const char	*pmic_name;
> +	int		fab_id;
> +	int		tp_rev;

int and not u32 or u64?

But again, who uses this module?  If it's only good for a single line in
the kernel log, that feels like a huge waste to me.

thanks,

greg k-h

Re: [PATCH 9/9] soc/qcom: Add REVID driver

[Konrad Dybcio]

Hi Greg, thanks for your review!

>Why do we need this noise in the kernel log?

I guess it could be left there as a debug print? Knowing your hardware
revision seems like a good, but yeah, not a necessary thing.

>You can drop the GPL boilerplate text and add a proper SPDX line at the
>top.

Seems I only did that in my other local tree.. whoops!

>Drivers should always use dev_err() and friends, as you have access to a
>struct device * always.  Please fix up the driver here to use that api
>instead, no pr_* should be needed at all.

Will do.

>Horrible global symbol name.  Who calls this?

Welcome to development on qcom platforms :D

>This is the last patch in
>the series, so if there is no user for this, please don't export it.

Other downstream drivers make use of it.. need to get this up first, sorry :V

>Why do you need a .h file in the include directory if only a single .c
>file needs it?  Just put that info in the .c file itself.

Again, other downstream drivers which some people and I intend to
bring to upstream standards use that to access the PMIC model/hw revision.

>But again, who uses this module?  If it's only good for a single line in
>the kernel log, that feels like a huge waste to me.

downstream-kernel-dir$ rg -l qpnp-revid.h | wc -l
25

So yeah, quite a bunch of other qcom-specific drivers.

I'll try to fix these and send a v2.

Regards
Konrad

Re: [PATCH 9/9] soc/qcom: Add REVID driver

[Greg Kroah-Hartman]

On Sun, Jul 26, 2020 at 01:40:46PM +0200, Konrad Dybcio wrote:
> >Horrible global symbol name.  Who calls this?
> 
> Welcome to development on qcom platforms :D

Yeah, I have seen :(

> >This is the last patch in
> >the series, so if there is no user for this, please don't export it.
> 
> Other downstream drivers make use of it.. need to get this up first, sorry :V

Then use a proper name for it, with a correct prefix, and export it with
EXPORT_SYMBOL_GPL() please.

> >Why do you need a .h file in the include directory if only a single .c
> >file needs it?  Just put that info in the .c file itself.
> 
> Again, other downstream drivers which some people and I intend to
> bring to upstream standards use that to access the PMIC model/hw revision.

But all of those defines are not needed, just the function name, right?

> >But again, who uses this module?  If it's only good for a single line in
> >the kernel log, that feels like a huge waste to me.
> 
> downstream-kernel-dir$ rg -l qpnp-revid.h | wc -l
> 25
> 
> So yeah, quite a bunch of other qcom-specific drivers.
> 
> I'll try to fix these and send a v2.

Hold off on this and wait until there is a real user for it, as we can
not take api additions without a user, otherwise we will just come along
and delete the code.

Submit it as part of a series that actually uses the function, so we can
at least see if the function makes sense to be used in that way as well,
right now we have no clue.

thanks,

greg k-h

Re: [PATCH 9/9] soc/qcom: Add REVID driver

[Rob Herring]

On Sun, Jul 26, 2020 at 01:12:06PM +0200, Konrad Dybcio wrote:
> From: Xiaozhe Shi <xiaozhes@codeaurora.org>
> 
> Add the REVID device driver. The REVID driver will print out the PMIC
> revision at probe time.
> 
> Signed-off-by: Xiaozhe Shi <xiaozhes@codeaurora.org>
> [konradybcio@gmail.com: Fast-forward the driver from kernel 4.14 to 5.8,
> convert binding to yaml]
> Signed-off-by: Konrad Dybcio <konradybcio@gmail.com>
> ---
>  .../bindings/soc/qcom/qcom,qpnp-revid.yaml    |  38 ++

Bindings should be a separate patch. checkpatch.pl will tell you this.

>  drivers/soc/qcom/Kconfig                      |   9 +
>  drivers/soc/qcom/Makefile                     |   1 +
>  drivers/soc/qcom/qpnp-revid.c                 | 288 ++++++++++++++
>  include/linux/qpnp/qpnp-revid.h               | 369 ++++++++++++++++++
>  5 files changed, 705 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,qpnp-revid.yaml
>  create mode 100644 drivers/soc/qcom/qpnp-revid.c
>  create mode 100644 include/linux/qpnp/qpnp-revid.h

Re: [PATCH 9/9] soc/qcom: Add REVID driver

[Rob Herring]

On Sun, 26 Jul 2020 13:12:06 +0200, Konrad Dybcio wrote:
> From: Xiaozhe Shi <xiaozhes@codeaurora.org>
> 
> Add the REVID device driver. The REVID driver will print out the PMIC
> revision at probe time.
> 
> Signed-off-by: Xiaozhe Shi <xiaozhes@codeaurora.org>
> [konradybcio@gmail.com: Fast-forward the driver from kernel 4.14 to 5.8,
> convert binding to yaml]
> Signed-off-by: Konrad Dybcio <konradybcio@gmail.com>
> ---
>  .../bindings/soc/qcom/qcom,qpnp-revid.yaml    |  38 ++
>  drivers/soc/qcom/Kconfig                      |   9 +
>  drivers/soc/qcom/Makefile                     |   1 +
>  drivers/soc/qcom/qpnp-revid.c                 | 288 ++++++++++++++
>  include/linux/qpnp/qpnp-revid.h               | 369 ++++++++++++++++++
>  5 files changed, 705 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,qpnp-revid.yaml
>  create mode 100644 drivers/soc/qcom/qpnp-revid.c
>  create mode 100644 include/linux/qpnp/qpnp-revid.h
> 


My bot found errors running 'make dt_binding_check' on your patch:

Documentation/devicetree/bindings/soc/qcom/qcom,qpnp-revid.yaml:  while scanning a block scalar
  in "<unicode string>", line 22, column 18
found a tab character where an indentation space is expected
  in "<unicode string>", line 24, column 1
Documentation/devicetree/bindings/Makefile:20: recipe for target 'Documentation/devicetree/bindings/soc/qcom/qcom,qpnp-revid.example.dts' failed
make[1]: *** [Documentation/devicetree/bindings/soc/qcom/qcom,qpnp-revid.example.dts] Error 1
make[1]: *** Waiting for unfinished jobs....
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/soc/qcom/qcom,qpnp-revid.yaml: ignoring, error parsing file
warning: no schema found in file: ./Documentation/devicetree/bindings/soc/qcom/qcom,qpnp-revid.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/soc/qcom/qcom,qpnp-revid.yaml: ignoring, error parsing file
warning: no schema found in file: ./Documentation/devicetree/bindings/soc/qcom/qcom,qpnp-revid.yaml
Makefile:1347: recipe for target 'dt_binding_check' failed
make: *** [dt_binding_check] Error 2


See https://patchwork.ozlabs.org/patch/1336467

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure dt-schema is up to date:

pip3 install git+https://github.com/devicetree-org/dt-schema.git@master --upgrade

Please check and re-submit.