Holes management in syscon driver

Holes management in syscon driver

[Maxime Coquelin]

Hi Dong, all,

We use syscon driver on our STi platforms to manage our system config registers.
We declare one syscon instance per sysconf bank.

The problem we are facing is that these banks have holes, and when using regmap's debugfs interface to dump the registers, we get imprecise aborts.

My first idea would be do implement the .readable_reg and .writeable_reg callbacks offered by regmap in syscon driver, and provide ranges where there are registers via DT.
But I am not sure how would look the DT bindings.

What is your view on this?
Are other syscon users having the same issue?

Regards,
Maxime

[RFC PATCH 0/2] mfd: syscon: declare holes in syscon register maps

[Seraphin Bonnaffe]

In reply to mail: Holes management in syscon driver. 

Hi Dong, Maxime, Lee, all,

Regmap also offers .rd_table and .wr_table structures that can be used to
specify valid ranges within agiven regmap configuration.

This patch uses these structures to declare holes in a syscon instance.
It takes the description from DT, and fills in the structures accordingly.

Can I have your opinion on this implementation ?

Thanks and Regards,
Seraphin

Seraphin Bonnaffe (2):
  mfd: syscon: Document new DT binding "holes"
  mfd: syscon: specify rd_table and wr_table from DT

 Documentation/devicetree/bindings/mfd/syscon.txt |  5 ++++
 drivers/mfd/syscon.c                             | 35 ++++++++++++++++++++++++
 2 files changed, 40 insertions(+)

-- 
1.9.1

[RFC PATCH 1/2] mfd: syscon: Document new DT binding "holes"

[Seraphin Bonnaffe]

This patch adds DT bindings to declare holes in syscon register maps.

Signed-off-by: Seraphin Bonnaffe <seraphin.bonnaffe@st.com>
---
 Documentation/devicetree/bindings/mfd/syscon.txt | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/syscon.txt b/Documentation/devicetree/bindings/mfd/syscon.txt
index fe8150b..80ab9ca 100644
--- a/Documentation/devicetree/bindings/mfd/syscon.txt
+++ b/Documentation/devicetree/bindings/mfd/syscon.txt
@@ -13,8 +13,13 @@ Required properties:
 - compatible: Should contain "syscon".
 - reg: the register region can be accessed from syscon
 
+Optional properties:
+- holes: Register regions that cannot be accessed within reg range.
+	 Each hole is described with its offset address, followed by its size.
+
 Examples:
 gpr: iomuxc-gpr at 020e0000 {
 	compatible = "fsl,imx6q-iomuxc-gpr", "syscon";
 	reg = <0x020e0000 0x38>;
+	holes = <0x000c 0x8 0x020 0x4>;
 };
-- 
1.9.1

[RFC PATCH 2/2] mfd: syscon: specify rd_table and wr_table from DT

[Seraphin Bonnaffe]

The syscon driver is based on regmap, which offers the possibility to
declare registers as readable/writable or not, thanks to rd_table and
wr_table.

This patch takes register map's holes description from DT, and fills in
the rd_table and wr_table of the corresponding syson instance accordingly.

Signed-off-by: Seraphin Bonnaffe <seraphin.bonnaffe@st.com>
---
 drivers/mfd/syscon.c | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/drivers/mfd/syscon.c b/drivers/mfd/syscon.c
index ca15878..2bfb45d 100644
--- a/drivers/mfd/syscon.c
+++ b/drivers/mfd/syscon.c
@@ -125,9 +125,15 @@ static int syscon_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct syscon_platform_data *pdata = dev_get_platdata(dev);
+	struct device_node *np = dev->of_node;
+	struct regmap_access_table *syscon_rw_table;
+	struct regmap_range *holes;
 	struct syscon *syscon;
 	struct resource *res;
 	void __iomem *base;
+	const __be32 *hole_prop;
+	u32 min, max, size;
+	u32 i, hlen, ngaps;
 
 	syscon = devm_kzalloc(dev, sizeof(*syscon), GFP_KERNEL);
 	if (!syscon)
@@ -141,6 +147,35 @@ static int syscon_probe(struct platform_device *pdev)
 	if (!base)
 		return -ENOMEM;
 
+	hole_prop = of_get_property(np, "holes", &hlen);
+	if (hole_prop) {
+		hlen /= sizeof(*hole_prop);
+		ngaps = hlen / 2;
+
+		holes =  devm_kzalloc(dev, ngaps * sizeof(*holes), GFP_KERNEL);
+		if (!holes)
+			return -ENOMEM;
+
+		for (i = 0; i < ngaps; i++) {
+			min = (u32)of_read_number(&hole_prop[i * 2], 1);
+			size = (u32)of_read_number(&hole_prop[i * 2 + 1], 1);
+			max = min + size - 1;
+
+			holes[i].range_min = min;
+			holes[i].range_max = max;
+		}
+		syscon_rw_table = devm_kzalloc(dev, sizeof(*syscon_rw_table),
+					       GFP_KERNEL);
+		if (!syscon_rw_table)
+			return -ENOMEM;
+
+		syscon_rw_table->no_ranges = holes;
+		syscon_rw_table->n_no_ranges = ngaps;
+
+		syscon_regmap_config.rd_table = syscon_rw_table;
+		syscon_regmap_config.wr_table = syscon_rw_table;
+	}
+
 	syscon_regmap_config.max_register = res->end - res->start - 3;
 	if (pdata)
 		syscon_regmap_config.name = pdata->label;
-- 
1.9.1

[RFC PATCH 0/2] mfd: syscon: declare holes in syscon register maps

[Maxime Coquelin]

Hi Seraphin,

On 10/07/2014 03:22 PM, Seraphin Bonnaffe wrote:
> In reply to mail: Holes management in syscon driver.
> 
> Hi Dong, Maxime, Lee, all,
> 
> Regmap also offers .rd_table and .wr_table structures that can be used to
> specify valid ranges within agiven regmap configuration.
> 
> This patch uses these structures to declare holes in a syscon instance.
> It takes the description from DT, and fills in the structures accordingly.
> 
> Can I have your opinion on this implementation ?

As discussed face to face, it looks fine to me.

But we would like green light in the new "holes" property introduced.

Thanks,
Maxime

> 
> Thanks and Regards,
> Seraphin
> 
> Seraphin Bonnaffe (2):
>    mfd: syscon: Document new DT binding "holes"
>    mfd: syscon: specify rd_table and wr_table from DT
> 
>   Documentation/devicetree/bindings/mfd/syscon.txt |  5 ++++
>   drivers/mfd/syscon.c                             | 35 ++++++++++++++++++++++++
>   2 files changed, 40 insertions(+)
>