Holes management in syscon driver

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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

Signed-off-by: Seraphin Bonnaffe <redacted>
---
 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@020e0000 {
 	compatible = "fsl,imx6q-iomuxc-gpr", "syscon";
 	reg = <0x020e0000 0x38>;
+	holes = <0x000c 0x8 0x020 0x4>;
 };

-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

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 <redacted>
---
 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

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(+)