diff mbox series

[v4,10/21] PM / devfreq: rockchip-dfi: Add RK3568 support

Message ID 20230505113856.463650-11-s.hauer@pengutronix.de (mailing list archive)
State New, archived
Headers show
Series Add perf support to the rockchip-dfi driver | expand

Commit Message

Sascha Hauer May 5, 2023, 11:38 a.m. UTC 
This adds RK3568 support to the DFI driver. The driver itself doesn't
need a change, only initialization differs from the currently supported
RK3399.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/devfreq/event/rockchip-dfi.c | 24 ++++++++++++++++++++++++
 include/soc/rockchip/rk3568_grf.h    | 12 ++++++++++++
 2 files changed, 36 insertions(+)
 create mode 100644 include/soc/rockchip/rk3568_grf.h

Comments

Jonathan Cameron May 16, 2023, 4:04 p.m. UTC | #1 
On Fri,  5 May 2023 13:38:45 +0200
Sascha Hauer <s.hauer@pengutronix.de> wrote:

> This adds RK3568 support to the DFI driver. The driver itself doesn't
> need a change, only initialization differs from the currently supported
> RK3399.
I'm not sure on distinction between driver and init (which is part of driver)
so that description might want to just say

Only initialization differs from currently supported RK3399.

and leave it at that!

> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  drivers/devfreq/event/rockchip-dfi.c | 24 ++++++++++++++++++++++++
>  include/soc/rockchip/rk3568_grf.h    | 12 ++++++++++++
>  2 files changed, 36 insertions(+)
>  create mode 100644 include/soc/rockchip/rk3568_grf.h
> 
> diff --git a/drivers/devfreq/event/rockchip-dfi.c b/drivers/devfreq/event/rockchip-dfi.c
> index 035984d3c7b01..78cb594bd2a81 100644
> --- a/drivers/devfreq/event/rockchip-dfi.c
> +++ b/drivers/devfreq/event/rockchip-dfi.c
> @@ -23,6 +23,7 @@
>  
>  #include <soc/rockchip/rockchip_grf.h>
>  #include <soc/rockchip/rk3399_grf.h>
> +#include <soc/rockchip/rk3568_grf.h>
>  
>  #define DMC_MAX_CHANNELS	2
>  
> @@ -209,6 +210,24 @@ static int rk3399_dfi_init(struct rockchip_dfi *dfi)
>  	return 0;
>  };
>  
> +static int rk3568_dfi_init(struct rockchip_dfi *dfi)
> +{
> +	struct regmap *regmap_pmu = dfi->regmap_pmu;
> +	u32 reg2, reg3;
> +
> +	regmap_read(regmap_pmu, RK3568_PMUGRF_OS_REG2, &reg2);
> +	regmap_read(regmap_pmu, RK3568_PMUGRF_OS_REG3, &reg3);
> +
> +	dfi->ddr_type = FIELD_GET(RK3568_PMUGRF_OS_REG2_DRAMTYPE_INFO, reg2);
> +
> +	if (FIELD_GET(RK3568_PMUGRF_OS_REG3_SYSREG_VERSION, reg3) >= 0x3)
> +		dfi->ddr_type |= FIELD_GET(RK3568_PMUGRF_OS_REG3_DRAMTYPE_INFO_V3, reg3) << 3;

This is unusual enough that I'd suggest some comments to say how
the bits are distributed across the two registers.

> +
> +	dfi->channel_mask = 1;
GENMASK(0, 0);
Or just use a count as that still looks fine to me.  Maybe that will change!


> +
> +	return 0;
> +};
> +
>  struct rockchip_dfi_devtype_data {
>  	int (*init)(struct rockchip_dfi *dfi);
>  };
> @@ -217,8 +236,13 @@ static struct rockchip_dfi_devtype_data rk3399_devtype_data = {
>  	.init = rk3399_dfi_init,
>  };
>  
> +static struct rockchip_dfi_devtype_data rk3568_devtype_data = {
> +	.init = rk3568_dfi_init,
> +};
> +
>  static const struct of_device_id rockchip_dfi_id_match[] = {
>  	{ .compatible = "rockchip,rk3399-dfi", .data = &rk3399_devtype_data },
> +	{ .compatible = "rockchip,rk3568-dfi", .data = &rk3568_devtype_data },
>  	{ },
>  };
>  MODULE_DEVICE_TABLE(of, rockchip_dfi_id_match);
> diff --git a/include/soc/rockchip/rk3568_grf.h b/include/soc/rockchip/rk3568_grf.h
> new file mode 100644
> index 0000000000000..575584e9d8834
> --- /dev/null
> +++ b/include/soc/rockchip/rk3568_grf.h
> @@ -0,0 +1,12 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +#ifndef __SOC_RK3568_GRF_H
> +#define __SOC_RK3568_GRF_H
> +
> +#define RK3568_PMUGRF_OS_REG2		0x208
> +#define RK3568_PMUGRF_OS_REG2_DRAMTYPE_INFO		GENMASK(15, 13)
> +
> +#define RK3568_PMUGRF_OS_REG3		0x20c
> +#define RK3568_PMUGRF_OS_REG3_DRAMTYPE_INFO_V3		GENMASK(13, 12)
> +#define RK3568_PMUGRF_OS_REG3_SYSREG_VERSION		GENMASK(31, 28)
> +
> +#endif /* __SOC_RK3568_GRF_H */
Sascha Hauer May 17, 2023, 11:38 a.m. UTC | #2 
On Tue, May 16, 2023 at 05:04:35PM +0100, Jonathan Cameron wrote:
> On Fri,  5 May 2023 13:38:45 +0200
> Sascha Hauer <s.hauer@pengutronix.de> wrote:
> 
> > This adds RK3568 support to the DFI driver. The driver itself doesn't
> > need a change, only initialization differs from the currently supported
> > RK3399.
> I'm not sure on distinction between driver and init (which is part of driver)
> so that description might want to just say
> 
> Only initialization differs from currently supported RK3399.
> 
> and leave it at that!
> 
> > 
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > ---
> >  drivers/devfreq/event/rockchip-dfi.c | 24 ++++++++++++++++++++++++
> >  include/soc/rockchip/rk3568_grf.h    | 12 ++++++++++++
> >  2 files changed, 36 insertions(+)
> >  create mode 100644 include/soc/rockchip/rk3568_grf.h
> > 
> > diff --git a/drivers/devfreq/event/rockchip-dfi.c b/drivers/devfreq/event/rockchip-dfi.c
> > index 035984d3c7b01..78cb594bd2a81 100644
> > --- a/drivers/devfreq/event/rockchip-dfi.c
> > +++ b/drivers/devfreq/event/rockchip-dfi.c
> > @@ -23,6 +23,7 @@
> >  
> >  #include <soc/rockchip/rockchip_grf.h>
> >  #include <soc/rockchip/rk3399_grf.h>
> > +#include <soc/rockchip/rk3568_grf.h>
> >  
> >  #define DMC_MAX_CHANNELS	2
> >  
> > @@ -209,6 +210,24 @@ static int rk3399_dfi_init(struct rockchip_dfi *dfi)
> >  	return 0;
> >  };
> >  
> > +static int rk3568_dfi_init(struct rockchip_dfi *dfi)
> > +{
> > +	struct regmap *regmap_pmu = dfi->regmap_pmu;
> > +	u32 reg2, reg3;
> > +
> > +	regmap_read(regmap_pmu, RK3568_PMUGRF_OS_REG2, &reg2);
> > +	regmap_read(regmap_pmu, RK3568_PMUGRF_OS_REG3, &reg3);
> > +
> > +	dfi->ddr_type = FIELD_GET(RK3568_PMUGRF_OS_REG2_DRAMTYPE_INFO, reg2);
> > +
> > +	if (FIELD_GET(RK3568_PMUGRF_OS_REG3_SYSREG_VERSION, reg3) >= 0x3)
> > +		dfi->ddr_type |= FIELD_GET(RK3568_PMUGRF_OS_REG3_DRAMTYPE_INFO_V3, reg3) << 3;
> 
> This is unusual enough that I'd suggest some comments to say how
> the bits are distributed across the two registers.

I'd say in version 3 of the register structure they realized that they
need two bits more for the ddr_type and have put them in BIT(12) and
BIT(13) of RK3568_PMUGRF_OS_REG3. I think the code makes that
sufficiently clear and apart from this code taken from the downstream
kernel I don't have any documentation of these registers.

Sascha
Jonathan Cameron May 17, 2023, 2:46 p.m. UTC | #3 
> > > +
> > > +	regmap_read(regmap_pmu, RK3568_PMUGRF_OS_REG2, &reg2);
> > > +	regmap_read(regmap_pmu, RK3568_PMUGRF_OS_REG3, &reg3);
> > > +
> > > +	dfi->ddr_type = FIELD_GET(RK3568_PMUGRF_OS_REG2_DRAMTYPE_INFO, reg2);
> > > +
> > > +	if (FIELD_GET(RK3568_PMUGRF_OS_REG3_SYSREG_VERSION, reg3) >= 0x3)
> > > +		dfi->ddr_type |= FIELD_GET(RK3568_PMUGRF_OS_REG3_DRAMTYPE_INFO_V3, reg3) << 3;  
> > 
> > This is unusual enough that I'd suggest some comments to say how
> > the bits are distributed across the two registers.  
> 
> I'd say in version 3 of the register structure they realized that they
> need two bits more for the ddr_type and have put them in BIT(12) and
> BIT(13) of RK3568_PMUGRF_OS_REG3. I think the code makes that
> sufficiently clear and apart from this code taken from the downstream
> kernel I don't have any documentation of these registers.
> 
> Sascha
OK, that's fine.

Jonathan

>
diff mbox series

Patch

diff --git a/drivers/devfreq/event/rockchip-dfi.c b/drivers/devfreq/event/rockchip-dfi.c
index 035984d3c7b01..78cb594bd2a81 100644
--- a/drivers/devfreq/event/rockchip-dfi.c
+++ b/drivers/devfreq/event/rockchip-dfi.c
@@ -23,6 +23,7 @@ 
 
 #include <soc/rockchip/rockchip_grf.h>
 #include <soc/rockchip/rk3399_grf.h>
+#include <soc/rockchip/rk3568_grf.h>
 
 #define DMC_MAX_CHANNELS	2
 
@@ -209,6 +210,24 @@  static int rk3399_dfi_init(struct rockchip_dfi *dfi)
 	return 0;
 };
 
+static int rk3568_dfi_init(struct rockchip_dfi *dfi)
+{
+	struct regmap *regmap_pmu = dfi->regmap_pmu;
+	u32 reg2, reg3;
+
+	regmap_read(regmap_pmu, RK3568_PMUGRF_OS_REG2, &reg2);
+	regmap_read(regmap_pmu, RK3568_PMUGRF_OS_REG3, &reg3);
+
+	dfi->ddr_type = FIELD_GET(RK3568_PMUGRF_OS_REG2_DRAMTYPE_INFO, reg2);
+
+	if (FIELD_GET(RK3568_PMUGRF_OS_REG3_SYSREG_VERSION, reg3) >= 0x3)
+		dfi->ddr_type |= FIELD_GET(RK3568_PMUGRF_OS_REG3_DRAMTYPE_INFO_V3, reg3) << 3;
+
+	dfi->channel_mask = 1;
+
+	return 0;
+};
+
 struct rockchip_dfi_devtype_data {
 	int (*init)(struct rockchip_dfi *dfi);
 };
@@ -217,8 +236,13 @@  static struct rockchip_dfi_devtype_data rk3399_devtype_data = {
 	.init = rk3399_dfi_init,
 };
 
+static struct rockchip_dfi_devtype_data rk3568_devtype_data = {
+	.init = rk3568_dfi_init,
+};
+
 static const struct of_device_id rockchip_dfi_id_match[] = {
 	{ .compatible = "rockchip,rk3399-dfi", .data = &rk3399_devtype_data },
+	{ .compatible = "rockchip,rk3568-dfi", .data = &rk3568_devtype_data },
 	{ },
 };
 MODULE_DEVICE_TABLE(of, rockchip_dfi_id_match);
diff --git a/include/soc/rockchip/rk3568_grf.h b/include/soc/rockchip/rk3568_grf.h
new file mode 100644
index 0000000000000..575584e9d8834
--- /dev/null
+++ b/include/soc/rockchip/rk3568_grf.h
@@ -0,0 +1,12 @@ 
+/* SPDX-License-Identifier: GPL-2.0+ */
+#ifndef __SOC_RK3568_GRF_H
+#define __SOC_RK3568_GRF_H
+
+#define RK3568_PMUGRF_OS_REG2		0x208
+#define RK3568_PMUGRF_OS_REG2_DRAMTYPE_INFO		GENMASK(15, 13)
+
+#define RK3568_PMUGRF_OS_REG3		0x20c
+#define RK3568_PMUGRF_OS_REG3_DRAMTYPE_INFO_V3		GENMASK(13, 12)
+#define RK3568_PMUGRF_OS_REG3_SYSREG_VERSION		GENMASK(31, 28)
+
+#endif /* __SOC_RK3568_GRF_H */