This series cleans up a few issues in the system controller driver, and
then expands the exported regmap to support one of the pairs of LDOs
built in to the D1 SoC.
Eventually, we will need to update the SRAM region claiming API so
ownership can be swapped back and forth by consumer drivers. This will
be necessary for uploading firmware to the R329/D1 DSPs, but it is not
needed for initial bringup.
Changes in v2:
- New patch to first clean up the binding
Samuel Holland (10):
dt-bindings: sram: sunxi-sram: Clean up the compatible lists
dt-bindings: sram: sunxi-sram: Add D1 compatible string
soc: sunxi: sram: Actually claim SRAM regions
soc: sunxi: sram: Prevent the driver from being unbound
soc: sunxi: sram: Fix probe function ordering issues
soc: sunxi: sram: Fix debugfs info for A64 SRAM C
soc: sunxi: sram: Return void from the release function
soc: sunxi: sram: Save a pointer to the OF match data
soc: sunxi: sram: Export the LDO control register
soc: sunxi: sram: Add support for the D1 system control
.../allwinner,sun4i-a10-system-control.yaml | 82 ++++++++-----------
drivers/soc/sunxi/sunxi_sram.c | 74 +++++++++--------
include/linux/soc/sunxi/sunxi_sram.h | 2 +-
3 files changed, 73 insertions(+), 85 deletions(-)
--
2.35.1
Use enumerations where appropriate to combine "const" choices and
deduplicate fallback compatible strings.
Suggested-by: Krzysztof Kozlowski <[email protected]>
Signed-off-by: Samuel Holland <[email protected]>
---
Changes in v2:
- New patch to first clean up the binding
.../allwinner,sun4i-a10-system-control.yaml | 81 +++++++------------
1 file changed, 31 insertions(+), 50 deletions(-)
diff --git a/Documentation/devicetree/bindings/sram/allwinner,sun4i-a10-system-control.yaml b/Documentation/devicetree/bindings/sram/allwinner,sun4i-a10-system-control.yaml
index 1c426c211e36..5055c9081059 100644
--- a/Documentation/devicetree/bindings/sram/allwinner,sun4i-a10-system-control.yaml
+++ b/Documentation/devicetree/bindings/sram/allwinner,sun4i-a10-system-control.yaml
@@ -24,32 +24,30 @@ properties:
compatible:
oneOf:
- - const: allwinner,sun4i-a10-sram-controller
+ - enum:
+ - allwinner,sun4i-a10-sram-controller
+ - allwinner,sun50i-a64-sram-controller
deprecated: true
- - const: allwinner,sun4i-a10-system-control
- - const: allwinner,sun5i-a13-system-control
+ - enum:
+ - allwinner,sun4i-a10-system-control
+ - allwinner,sun5i-a13-system-control
+ - allwinner,sun8i-a23-system-control
+ - allwinner,sun8i-h3-system-control
+ - allwinner,sun50i-a64-system-control
+ - allwinner,sun50i-h5-system-control
+ - allwinner,sun50i-h616-system-control
- items:
- - const: allwinner,sun7i-a20-system-control
+ - enum:
+ - allwinner,suniv-f1c100s-system-control
+ - allwinner,sun7i-a20-system-control
+ - allwinner,sun8i-r40-system-control
- const: allwinner,sun4i-a10-system-control
- - const: allwinner,sun8i-a23-system-control
- - const: allwinner,sun8i-h3-system-control
- items:
- const: allwinner,sun8i-v3s-system-control
- const: allwinner,sun8i-h3-system-control
- - items:
- - const: allwinner,sun8i-r40-system-control
- - const: allwinner,sun4i-a10-system-control
- - const: allwinner,sun50i-a64-sram-controller
- deprecated: true
- - const: allwinner,sun50i-a64-system-control
- - const: allwinner,sun50i-h5-system-control
- items:
- const: allwinner,sun50i-h6-system-control
- const: allwinner,sun50i-a64-system-control
- - items:
- - const: allwinner,suniv-f1c100s-system-control
- - const: allwinner,sun4i-a10-system-control
- - const: allwinner,sun50i-h616-system-control
reg:
maxItems: 1
@@ -76,43 +74,26 @@ patternProperties:
- const: allwinner,sun4i-a10-sram-d
- const: allwinner,sun50i-a64-sram-c
- items:
- - const: allwinner,sun5i-a13-sram-a3-a4
- - const: allwinner,sun4i-a10-sram-a3-a4
- - items:
- - const: allwinner,sun7i-a20-sram-a3-a4
+ - enum:
+ - allwinner,sun5i-a13-sram-a3-a4
+ - allwinner,sun7i-a20-sram-a3-a4
- const: allwinner,sun4i-a10-sram-a3-a4
- items:
- - const: allwinner,sun5i-a13-sram-c1
- - const: allwinner,sun4i-a10-sram-c1
- - items:
- - const: allwinner,sun7i-a20-sram-c1
- - const: allwinner,sun4i-a10-sram-c1
- - items:
- - const: allwinner,sun8i-a23-sram-c1
- - const: allwinner,sun4i-a10-sram-c1
- - items:
- - const: allwinner,sun8i-h3-sram-c1
+ - enum:
+ - allwinner,sun5i-a13-sram-c1
+ - allwinner,sun7i-a20-sram-c1
+ - allwinner,sun8i-a23-sram-c1
+ - allwinner,sun8i-h3-sram-c1
+ - allwinner,sun8i-r40-sram-c1
+ - allwinner,sun50i-a64-sram-c1
+ - allwinner,sun50i-h5-sram-c1
+ - allwinner,sun50i-h6-sram-c1
- const: allwinner,sun4i-a10-sram-c1
- items:
- - const: allwinner,sun8i-r40-sram-c1
- - const: allwinner,sun4i-a10-sram-c1
- - items:
- - const: allwinner,sun50i-a64-sram-c1
- - const: allwinner,sun4i-a10-sram-c1
- - items:
- - const: allwinner,sun50i-h5-sram-c1
- - const: allwinner,sun4i-a10-sram-c1
- - items:
- - const: allwinner,sun50i-h6-sram-c1
- - const: allwinner,sun4i-a10-sram-c1
- - items:
- - const: allwinner,sun5i-a13-sram-d
- - const: allwinner,sun4i-a10-sram-d
- - items:
- - const: allwinner,sun7i-a20-sram-d
- - const: allwinner,sun4i-a10-sram-d
- - items:
- - const: allwinner,suniv-f1c100s-sram-d
+ - enum:
+ - allwinner,suniv-f1c100s-sram-d
+ - allwinner,sun5i-a13-sram-d
+ - allwinner,sun7i-a20-sram-d
- const: allwinner,sun4i-a10-sram-d
- items:
- const: allwinner,sun50i-h6-sram-c
--
2.35.1
sunxi_sram_claim() checks the sram_desc->claimed flag before updating
the register, with the intent that only one device can claim a region.
However, this was ineffective because the flag was never set.
Fixes: 4af34b572a85 ("drivers: soc: sunxi: Introduce SoC driver to map SRAMs")
Reviewed-by: Jernej Skrabec <[email protected]>
Signed-off-by: Samuel Holland <[email protected]>
---
(no changes since v1)
drivers/soc/sunxi/sunxi_sram.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/soc/sunxi/sunxi_sram.c b/drivers/soc/sunxi/sunxi_sram.c
index a8f3876963a0..f3d3f9259df9 100644
--- a/drivers/soc/sunxi/sunxi_sram.c
+++ b/drivers/soc/sunxi/sunxi_sram.c
@@ -254,6 +254,7 @@ int sunxi_sram_claim(struct device *dev)
writel(val | ((device << sram_data->offset) & mask),
base + sram_data->reg);
+ sram_desc->claimed = true;
spin_unlock(&sram_lock);
return 0;
--
2.35.1
The labels were backward with respect to the register values. The SRAM
is mapped to the CPU when the register value is 1.
Fixes: 5e4fb6429761 ("drivers: soc: sunxi: add support for A64 and its SRAM C")
Acked-by: Jernej Skrabec <[email protected]>
Signed-off-by: Samuel Holland <[email protected]>
---
(no changes since v1)
drivers/soc/sunxi/sunxi_sram.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/soc/sunxi/sunxi_sram.c b/drivers/soc/sunxi/sunxi_sram.c
index 52d07bed7664..09754cd1d57d 100644
--- a/drivers/soc/sunxi/sunxi_sram.c
+++ b/drivers/soc/sunxi/sunxi_sram.c
@@ -78,8 +78,8 @@ static struct sunxi_sram_desc sun4i_a10_sram_d = {
static struct sunxi_sram_desc sun50i_a64_sram_c = {
.data = SUNXI_SRAM_DATA("C", 0x4, 24, 1,
- SUNXI_SRAM_MAP(0, 1, "cpu"),
- SUNXI_SRAM_MAP(1, 0, "de2")),
+ SUNXI_SRAM_MAP(1, 0, "cpu"),
+ SUNXI_SRAM_MAP(0, 1, "de2")),
};
static const struct of_device_id sunxi_sram_dt_ids[] = {
--
2.35.1
D1 has a single EMAC and some LDOs that need to be exported.
Acked-by: Jernej Skrabec <[email protected]>
Signed-off-by: Samuel Holland <[email protected]>
---
(no changes since v1)
drivers/soc/sunxi/sunxi_sram.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/soc/sunxi/sunxi_sram.c b/drivers/soc/sunxi/sunxi_sram.c
index 7e8dab0f0ec4..92f9186c1c42 100644
--- a/drivers/soc/sunxi/sunxi_sram.c
+++ b/drivers/soc/sunxi/sunxi_sram.c
@@ -294,6 +294,11 @@ static const struct sunxi_sramc_variant sun8i_h3_sramc_variant = {
.num_emac_clocks = 1,
};
+static const struct sunxi_sramc_variant sun20i_d1_sramc_variant = {
+ .num_emac_clocks = 1,
+ .has_ldo_ctrl = true,
+};
+
static const struct sunxi_sramc_variant sun50i_a64_sramc_variant = {
.num_emac_clocks = 1,
};
@@ -382,6 +387,10 @@ static const struct of_device_id sunxi_sram_dt_match[] = {
.compatible = "allwinner,sun8i-h3-system-control",
.data = &sun8i_h3_sramc_variant,
},
+ {
+ .compatible = "allwinner,sun20i-d1-system-control",
+ .data = &sun20i_d1_sramc_variant,
+ },
{
.compatible = "allwinner,sun50i-a64-sram-controller",
.data = &sun50i_a64_sramc_variant,
--
2.35.1
This driver exports a regmap tied to the platform device (as opposed to
a syscon, which exports a regmap tied to the OF node). Because of this,
the driver can never be unbound, as that would destroy the regmap. Use
builtin_platform_driver_probe() to enforce this limitation.
Fixes: 5828729bebbb ("soc: sunxi: export a regmap for EMAC clock reg on A64")
Reviewed-by: Jernej Skrabec <[email protected]>
Signed-off-by: Samuel Holland <[email protected]>
---
(no changes since v1)
drivers/soc/sunxi/sunxi_sram.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/soc/sunxi/sunxi_sram.c b/drivers/soc/sunxi/sunxi_sram.c
index f3d3f9259df9..a858a37fcdd4 100644
--- a/drivers/soc/sunxi/sunxi_sram.c
+++ b/drivers/soc/sunxi/sunxi_sram.c
@@ -330,7 +330,7 @@ static struct regmap_config sunxi_sram_emac_clock_regmap = {
.writeable_reg = sunxi_sram_regmap_accessible_reg,
};
-static int sunxi_sram_probe(struct platform_device *pdev)
+static int __init sunxi_sram_probe(struct platform_device *pdev)
{
struct dentry *d;
struct regmap *emac_clock;
@@ -410,9 +410,8 @@ static struct platform_driver sunxi_sram_driver = {
.name = "sunxi-sram",
.of_match_table = sunxi_sram_dt_match,
},
- .probe = sunxi_sram_probe,
};
-module_platform_driver(sunxi_sram_driver);
+builtin_platform_driver_probe(sunxi_sram_driver, sunxi_sram_probe);
MODULE_AUTHOR("Maxime Ripard <[email protected]>");
MODULE_DESCRIPTION("Allwinner sunXi SRAM Controller Driver");
--
2.35.1
There is no point in returning an error here, as the caller can do
nothing about it. In fact, all callers already ignore the return value.
Acked-by: Jernej Skrabec <[email protected]>
Signed-off-by: Samuel Holland <[email protected]>
---
(no changes since v1)
drivers/soc/sunxi/sunxi_sram.c | 8 +++-----
include/linux/soc/sunxi/sunxi_sram.h | 2 +-
2 files changed, 4 insertions(+), 6 deletions(-)
diff --git a/drivers/soc/sunxi/sunxi_sram.c b/drivers/soc/sunxi/sunxi_sram.c
index 09754cd1d57d..9622fd45f5e5 100644
--- a/drivers/soc/sunxi/sunxi_sram.c
+++ b/drivers/soc/sunxi/sunxi_sram.c
@@ -261,25 +261,23 @@ int sunxi_sram_claim(struct device *dev)
}
EXPORT_SYMBOL(sunxi_sram_claim);
-int sunxi_sram_release(struct device *dev)
+void sunxi_sram_release(struct device *dev)
{
const struct sunxi_sram_data *sram_data;
struct sunxi_sram_desc *sram_desc;
if (!dev || !dev->of_node)
- return -EINVAL;
+ return;
sram_data = sunxi_sram_of_parse(dev->of_node, NULL);
if (IS_ERR(sram_data))
- return -EINVAL;
+ return;
sram_desc = to_sram_desc(sram_data);
spin_lock(&sram_lock);
sram_desc->claimed = false;
spin_unlock(&sram_lock);
-
- return 0;
}
EXPORT_SYMBOL(sunxi_sram_release);
diff --git a/include/linux/soc/sunxi/sunxi_sram.h b/include/linux/soc/sunxi/sunxi_sram.h
index c5f663bba9c2..60e274d1b821 100644
--- a/include/linux/soc/sunxi/sunxi_sram.h
+++ b/include/linux/soc/sunxi/sunxi_sram.h
@@ -14,6 +14,6 @@
#define _SUNXI_SRAM_H_
int sunxi_sram_claim(struct device *dev);
-int sunxi_sram_release(struct device *dev);
+void sunxi_sram_release(struct device *dev);
#endif /* _SUNXI_SRAM_H_ */
--
2.35.1
Some newer Allwinner SoCs contain internal LDOs managed by a register in
the system control MMIO space. Export this from the regmap in addtion to
the EMAC register.
Use generic names now that the regmap is no longer EMAC-specific.
Acked-by: Jernej Skrabec <[email protected]>
Signed-off-by: Samuel Holland <[email protected]>
---
(no changes since v1)
drivers/soc/sunxi/sunxi_sram.c | 30 ++++++++++++++++--------------
1 file changed, 16 insertions(+), 14 deletions(-)
diff --git a/drivers/soc/sunxi/sunxi_sram.c b/drivers/soc/sunxi/sunxi_sram.c
index 7c6fb17cfe7f..7e8dab0f0ec4 100644
--- a/drivers/soc/sunxi/sunxi_sram.c
+++ b/drivers/soc/sunxi/sunxi_sram.c
@@ -283,6 +283,7 @@ EXPORT_SYMBOL(sunxi_sram_release);
struct sunxi_sramc_variant {
int num_emac_clocks;
+ bool has_ldo_ctrl;
};
static const struct sunxi_sramc_variant sun4i_a10_sramc_variant = {
@@ -302,25 +303,28 @@ static const struct sunxi_sramc_variant sun50i_h616_sramc_variant = {
};
#define SUNXI_SRAM_EMAC_CLOCK_REG 0x30
+#define SUNXI_SYS_LDO_CTRL_REG 0x150
+
static bool sunxi_sram_regmap_accessible_reg(struct device *dev,
unsigned int reg)
{
const struct sunxi_sramc_variant *variant = dev_get_drvdata(dev);
- if (reg < SUNXI_SRAM_EMAC_CLOCK_REG)
- return false;
- if (reg > SUNXI_SRAM_EMAC_CLOCK_REG + variant->num_emac_clocks * 4)
- return false;
+ if (reg >= SUNXI_SRAM_EMAC_CLOCK_REG &&
+ reg < SUNXI_SRAM_EMAC_CLOCK_REG + variant->num_emac_clocks * 4)
+ return true;
+ if (reg == SUNXI_SYS_LDO_CTRL_REG && variant->has_ldo_ctrl)
+ return true;
- return true;
+ return false;
}
-static struct regmap_config sunxi_sram_emac_clock_regmap = {
+static struct regmap_config sunxi_sram_regmap_config = {
.reg_bits = 32,
.val_bits = 32,
.reg_stride = 4,
/* last defined register */
- .max_register = SUNXI_SRAM_EMAC_CLOCK_REG + 4,
+ .max_register = SUNXI_SYS_LDO_CTRL_REG,
/* other devices have no business accessing other registers */
.readable_reg = sunxi_sram_regmap_accessible_reg,
.writeable_reg = sunxi_sram_regmap_accessible_reg,
@@ -328,9 +332,9 @@ static struct regmap_config sunxi_sram_emac_clock_regmap = {
static int __init sunxi_sram_probe(struct platform_device *pdev)
{
- struct regmap *emac_clock;
const struct sunxi_sramc_variant *variant;
struct device *dev = &pdev->dev;
+ struct regmap *regmap;
sram_dev = &pdev->dev;
@@ -344,12 +348,10 @@ static int __init sunxi_sram_probe(struct platform_device *pdev)
if (IS_ERR(base))
return PTR_ERR(base);
- if (variant->num_emac_clocks > 0) {
- emac_clock = devm_regmap_init_mmio(&pdev->dev, base,
- &sunxi_sram_emac_clock_regmap);
-
- if (IS_ERR(emac_clock))
- return PTR_ERR(emac_clock);
+ if (variant->num_emac_clocks || variant->has_ldo_ctrl) {
+ regmap = devm_regmap_init_mmio(dev, base, &sunxi_sram_regmap_config);
+ if (IS_ERR(regmap))
+ return PTR_ERR(regmap);
}
of_platform_populate(dev->of_node, NULL, NULL, dev);
--
2.35.1
D1 needs to export a register for managing some LDO regulators, so it
needs a unique compatible.
Signed-off-by: Samuel Holland <[email protected]>
---
(no changes since v1)
.../bindings/sram/allwinner,sun4i-a10-system-control.yaml | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/sram/allwinner,sun4i-a10-system-control.yaml b/Documentation/devicetree/bindings/sram/allwinner,sun4i-a10-system-control.yaml
index 5055c9081059..d64c1b28fb61 100644
--- a/Documentation/devicetree/bindings/sram/allwinner,sun4i-a10-system-control.yaml
+++ b/Documentation/devicetree/bindings/sram/allwinner,sun4i-a10-system-control.yaml
@@ -33,6 +33,7 @@ properties:
- allwinner,sun5i-a13-system-control
- allwinner,sun8i-a23-system-control
- allwinner,sun8i-h3-system-control
+ - allwinner,sun20i-d1-system-control
- allwinner,sun50i-a64-system-control
- allwinner,sun50i-h5-system-control
- allwinner,sun50i-h616-system-control
--
2.35.1
It is inefficient to match the compatible string every time the regmap
is accessed.
Acked-by: Jernej Skrabec <[email protected]>
Signed-off-by: Samuel Holland <[email protected]>
---
(no changes since v1)
drivers/soc/sunxi/sunxi_sram.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/soc/sunxi/sunxi_sram.c b/drivers/soc/sunxi/sunxi_sram.c
index 9622fd45f5e5..7c6fb17cfe7f 100644
--- a/drivers/soc/sunxi/sunxi_sram.c
+++ b/drivers/soc/sunxi/sunxi_sram.c
@@ -305,9 +305,7 @@ static const struct sunxi_sramc_variant sun50i_h616_sramc_variant = {
static bool sunxi_sram_regmap_accessible_reg(struct device *dev,
unsigned int reg)
{
- const struct sunxi_sramc_variant *variant;
-
- variant = of_device_get_match_data(dev);
+ const struct sunxi_sramc_variant *variant = dev_get_drvdata(dev);
if (reg < SUNXI_SRAM_EMAC_CLOCK_REG)
return false;
@@ -340,6 +338,8 @@ static int __init sunxi_sram_probe(struct platform_device *pdev)
if (!variant)
return -EINVAL;
+ dev_set_drvdata(dev, (struct sunxi_sramc_variant *)variant);
+
base = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(base))
return PTR_ERR(base);
--
2.35.1
Errors from debugfs are intended to be non-fatal, and should not prevent
the driver from probing.
Since debugfs file creation is treated as infallible, move it below the
parts of the probe function that can fail. This prevents an error
elsewhere in the probe function from causing the file to leak. Do the
same for the call to of_platform_populate().
Finally, checkpatch suggests an octal literal for the file permissions.
Fixes: 4af34b572a85 ("drivers: soc: sunxi: Introduce SoC driver to map SRAMs")
Fixes: 5828729bebbb ("soc: sunxi: export a regmap for EMAC clock reg on A64")
Reviewed-by: Jernej Skrabec <[email protected]>
Signed-off-by: Samuel Holland <[email protected]>
---
(no changes since v1)
drivers/soc/sunxi/sunxi_sram.c | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/drivers/soc/sunxi/sunxi_sram.c b/drivers/soc/sunxi/sunxi_sram.c
index a858a37fcdd4..52d07bed7664 100644
--- a/drivers/soc/sunxi/sunxi_sram.c
+++ b/drivers/soc/sunxi/sunxi_sram.c
@@ -332,9 +332,9 @@ static struct regmap_config sunxi_sram_emac_clock_regmap = {
static int __init sunxi_sram_probe(struct platform_device *pdev)
{
- struct dentry *d;
struct regmap *emac_clock;
const struct sunxi_sramc_variant *variant;
+ struct device *dev = &pdev->dev;
sram_dev = &pdev->dev;
@@ -346,13 +346,6 @@ static int __init sunxi_sram_probe(struct platform_device *pdev)
if (IS_ERR(base))
return PTR_ERR(base);
- of_platform_populate(pdev->dev.of_node, NULL, NULL, &pdev->dev);
-
- d = debugfs_create_file("sram", S_IRUGO, NULL, NULL,
- &sunxi_sram_fops);
- if (!d)
- return -ENOMEM;
-
if (variant->num_emac_clocks > 0) {
emac_clock = devm_regmap_init_mmio(&pdev->dev, base,
&sunxi_sram_emac_clock_regmap);
@@ -361,6 +354,10 @@ static int __init sunxi_sram_probe(struct platform_device *pdev)
return PTR_ERR(emac_clock);
}
+ of_platform_populate(dev->of_node, NULL, NULL, dev);
+
+ debugfs_create_file("sram", 0444, NULL, NULL, &sunxi_sram_fops);
+
return 0;
}
--
2.35.1
On 15/08/2022 07:57, Conor Dooley wrote:
> Hey Samuel,
> On 15/08/2022 05:12, Samuel Holland wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>
>> This series cleans up a few issues in the system controller driver, and
>> then expands the exported regmap to support one of the pairs of LDOs
>> built in to the D1 SoC.
>>
>> Eventually, we will need to update the SRAM region claiming API so
>> ownership can be swapped back and forth by consumer drivers. This will
>> be necessary for uploading firmware to the R329/D1 DSPs, but it is not
>> needed for initial bringup.
>
> I tried this series on top of both 6.0-rc1 & next, but I saw a shed
> load of errors from dtbs_check. I'll take a look at the rest of the
> series later on, but just FYI. I know I have seen patches going into
> different subsystems so I assume you're working on it. However, it
> would be nice to let us what all the dependencies for this series are
> dt-binding wise in the cover letter.
> I have been trying to get riscv down to 0 warnings and would not be
> in favour of accepting patches that go in the other direction.
> (This patchset currently produces about 75 warnings)
>
Ignore this, I read the wrong cover letter. Sorry.
Hey Samuel,
On 15/08/2022 05:12, Samuel Holland wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> This series cleans up a few issues in the system controller driver, and
> then expands the exported regmap to support one of the pairs of LDOs
> built in to the D1 SoC.
>
> Eventually, we will need to update the SRAM region claiming API so
> ownership can be swapped back and forth by consumer drivers. This will
> be necessary for uploading firmware to the R329/D1 DSPs, but it is not
> needed for initial bringup.
I tried this series on top of both 6.0-rc1 & next, but I saw a shed
load of errors from dtbs_check. I'll take a look at the rest of the
series later on, but just FYI. I know I have seen patches going into
different subsystems so I assume you're working on it. However, it
would be nice to let us what all the dependencies for this series are
dt-binding wise in the cover letter.
I have been trying to get riscv down to 0 warnings and would not be
in favour of accepting patches that go in the other direction.
(This patchset currently produces about 75 warnings)
Thanks,
Conor.
>
> Changes in v2:
> - New patch to first clean up the binding
>
> Samuel Holland (10):
> dt-bindings: sram: sunxi-sram: Clean up the compatible lists
> dt-bindings: sram: sunxi-sram: Add D1 compatible string
> soc: sunxi: sram: Actually claim SRAM regions
> soc: sunxi: sram: Prevent the driver from being unbound
> soc: sunxi: sram: Fix probe function ordering issues
> soc: sunxi: sram: Fix debugfs info for A64 SRAM C
> soc: sunxi: sram: Return void from the release function
> soc: sunxi: sram: Save a pointer to the OF match data
> soc: sunxi: sram: Export the LDO control register
> soc: sunxi: sram: Add support for the D1 system control
>
> .../allwinner,sun4i-a10-system-control.yaml | 82 ++++++++-----------
> drivers/soc/sunxi/sunxi_sram.c | 74 +++++++++--------
> include/linux/soc/sunxi/sunxi_sram.h | 2 +-
> 3 files changed, 73 insertions(+), 85 deletions(-)
>
> --
> 2.35.1
>
On Sun, 14 Aug 2022 23:12:38 -0500
Samuel Holland <[email protected]> wrote:
> Use enumerations where appropriate to combine "const" choices and
> deduplicate fallback compatible strings.
>
> Suggested-by: Krzysztof Kozlowski <[email protected]>
> Signed-off-by: Samuel Holland <[email protected]>
Verified to be a pure transformation.
Reviewed-by: Andre Przywara <[email protected]>
Cheers,
Andre
> ---
>
> Changes in v2:
> - New patch to first clean up the binding
>
> .../allwinner,sun4i-a10-system-control.yaml | 81 +++++++------------
> 1 file changed, 31 insertions(+), 50 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/sram/allwinner,sun4i-a10-system-control.yaml b/Documentation/devicetree/bindings/sram/allwinner,sun4i-a10-system-control.yaml
> index 1c426c211e36..5055c9081059 100644
> --- a/Documentation/devicetree/bindings/sram/allwinner,sun4i-a10-system-control.yaml
> +++ b/Documentation/devicetree/bindings/sram/allwinner,sun4i-a10-system-control.yaml
> @@ -24,32 +24,30 @@ properties:
>
> compatible:
> oneOf:
> - - const: allwinner,sun4i-a10-sram-controller
> + - enum:
> + - allwinner,sun4i-a10-sram-controller
> + - allwinner,sun50i-a64-sram-controller
> deprecated: true
> - - const: allwinner,sun4i-a10-system-control
> - - const: allwinner,sun5i-a13-system-control
> + - enum:
> + - allwinner,sun4i-a10-system-control
> + - allwinner,sun5i-a13-system-control
> + - allwinner,sun8i-a23-system-control
> + - allwinner,sun8i-h3-system-control
> + - allwinner,sun50i-a64-system-control
> + - allwinner,sun50i-h5-system-control
> + - allwinner,sun50i-h616-system-control
> - items:
> - - const: allwinner,sun7i-a20-system-control
> + - enum:
> + - allwinner,suniv-f1c100s-system-control
> + - allwinner,sun7i-a20-system-control
> + - allwinner,sun8i-r40-system-control
> - const: allwinner,sun4i-a10-system-control
> - - const: allwinner,sun8i-a23-system-control
> - - const: allwinner,sun8i-h3-system-control
> - items:
> - const: allwinner,sun8i-v3s-system-control
> - const: allwinner,sun8i-h3-system-control
> - - items:
> - - const: allwinner,sun8i-r40-system-control
> - - const: allwinner,sun4i-a10-system-control
> - - const: allwinner,sun50i-a64-sram-controller
> - deprecated: true
> - - const: allwinner,sun50i-a64-system-control
> - - const: allwinner,sun50i-h5-system-control
> - items:
> - const: allwinner,sun50i-h6-system-control
> - const: allwinner,sun50i-a64-system-control
> - - items:
> - - const: allwinner,suniv-f1c100s-system-control
> - - const: allwinner,sun4i-a10-system-control
> - - const: allwinner,sun50i-h616-system-control
>
> reg:
> maxItems: 1
> @@ -76,43 +74,26 @@ patternProperties:
> - const: allwinner,sun4i-a10-sram-d
> - const: allwinner,sun50i-a64-sram-c
> - items:
> - - const: allwinner,sun5i-a13-sram-a3-a4
> - - const: allwinner,sun4i-a10-sram-a3-a4
> - - items:
> - - const: allwinner,sun7i-a20-sram-a3-a4
> + - enum:
> + - allwinner,sun5i-a13-sram-a3-a4
> + - allwinner,sun7i-a20-sram-a3-a4
> - const: allwinner,sun4i-a10-sram-a3-a4
> - items:
> - - const: allwinner,sun5i-a13-sram-c1
> - - const: allwinner,sun4i-a10-sram-c1
> - - items:
> - - const: allwinner,sun7i-a20-sram-c1
> - - const: allwinner,sun4i-a10-sram-c1
> - - items:
> - - const: allwinner,sun8i-a23-sram-c1
> - - const: allwinner,sun4i-a10-sram-c1
> - - items:
> - - const: allwinner,sun8i-h3-sram-c1
> + - enum:
> + - allwinner,sun5i-a13-sram-c1
> + - allwinner,sun7i-a20-sram-c1
> + - allwinner,sun8i-a23-sram-c1
> + - allwinner,sun8i-h3-sram-c1
> + - allwinner,sun8i-r40-sram-c1
> + - allwinner,sun50i-a64-sram-c1
> + - allwinner,sun50i-h5-sram-c1
> + - allwinner,sun50i-h6-sram-c1
> - const: allwinner,sun4i-a10-sram-c1
> - items:
> - - const: allwinner,sun8i-r40-sram-c1
> - - const: allwinner,sun4i-a10-sram-c1
> - - items:
> - - const: allwinner,sun50i-a64-sram-c1
> - - const: allwinner,sun4i-a10-sram-c1
> - - items:
> - - const: allwinner,sun50i-h5-sram-c1
> - - const: allwinner,sun4i-a10-sram-c1
> - - items:
> - - const: allwinner,sun50i-h6-sram-c1
> - - const: allwinner,sun4i-a10-sram-c1
> - - items:
> - - const: allwinner,sun5i-a13-sram-d
> - - const: allwinner,sun4i-a10-sram-d
> - - items:
> - - const: allwinner,sun7i-a20-sram-d
> - - const: allwinner,sun4i-a10-sram-d
> - - items:
> - - const: allwinner,suniv-f1c100s-sram-d
> + - enum:
> + - allwinner,suniv-f1c100s-sram-d
> + - allwinner,sun5i-a13-sram-d
> + - allwinner,sun7i-a20-sram-d
> - const: allwinner,sun4i-a10-sram-d
> - items:
> - const: allwinner,sun50i-h6-sram-c
Am Montag, 15. August 2022, 06:12:38 CEST schrieb Samuel Holland:
> Use enumerations where appropriate to combine "const" choices and
> deduplicate fallback compatible strings.
>
> Suggested-by: Krzysztof Kozlowski <[email protected]>
> Signed-off-by: Samuel Holland <[email protected]>
Reviewed-by: Heiko Stuebner <[email protected]>
Am Montag, 15. August 2022, 06:12:46 CEST schrieb Samuel Holland:
> Some newer Allwinner SoCs contain internal LDOs managed by a register in
> the system control MMIO space. Export this from the regmap in addtion to
> the EMAC register.
>
> Use generic names now that the regmap is no longer EMAC-specific.
>
> Acked-by: Jernej Skrabec <[email protected]>
> Signed-off-by: Samuel Holland <[email protected]>
Reviewed-by: Heiko Stuebner <[email protected]>
Tested-by: Heiko Stuebner <[email protected]>
Am Montag, 15. August 2022, 06:12:39 CEST schrieb Samuel Holland:
> D1 needs to export a register for managing some LDO regulators, so it
> needs a unique compatible.
>
> Signed-off-by: Samuel Holland <[email protected]>
Reviewed-by: Heiko Stuebner <[email protected]>
Am Montag, 15. August 2022, 06:12:40 CEST schrieb Samuel Holland:
> sunxi_sram_claim() checks the sram_desc->claimed flag before updating
> the register, with the intent that only one device can claim a region.
> However, this was ineffective because the flag was never set.
>
> Fixes: 4af34b572a85 ("drivers: soc: sunxi: Introduce SoC driver to map SRAMs")
> Reviewed-by: Jernej Skrabec <[email protected]>
> Signed-off-by: Samuel Holland <[email protected]>
Reviewed-by: Heiko Stuebner <[email protected]>
Tested-by: Heiko Stuebner <[email protected]>
Am Montag, 15. August 2022, 06:12:45 CEST schrieb Samuel Holland:
> It is inefficient to match the compatible string every time the regmap
> is accessed.
>
> Acked-by: Jernej Skrabec <[email protected]>
> Signed-off-by: Samuel Holland <[email protected]>
Reviewed-by: Heiko Stuebner <[email protected]>
Tested-by: Heiko Stuebner <[email protected]>
Am Montag, 15. August 2022, 06:12:44 CEST schrieb Samuel Holland:
> There is no point in returning an error here, as the caller can do
> nothing about it. In fact, all callers already ignore the return value.
>
> Acked-by: Jernej Skrabec <[email protected]>
> Signed-off-by: Samuel Holland <[email protected]>
Reviewed-by: Heiko Stuebner <[email protected]>
Tested-by: Heiko Stuebner <[email protected]>
Am Montag, 15. August 2022, 06:12:41 CEST schrieb Samuel Holland:
> This driver exports a regmap tied to the platform device (as opposed to
> a syscon, which exports a regmap tied to the OF node). Because of this,
> the driver can never be unbound, as that would destroy the regmap. Use
> builtin_platform_driver_probe() to enforce this limitation.
>
> Fixes: 5828729bebbb ("soc: sunxi: export a regmap for EMAC clock reg on A64")
> Reviewed-by: Jernej Skrabec <[email protected]>
> Signed-off-by: Samuel Holland <[email protected]>
Reviewed-by: Heiko Stuebner <[email protected]>
Tested-by: Heiko Stuebner <[email protected]>
Am Montag, 15. August 2022, 06:12:42 CEST schrieb Samuel Holland:
> Errors from debugfs are intended to be non-fatal, and should not prevent
> the driver from probing.
>
> Since debugfs file creation is treated as infallible, move it below the
> parts of the probe function that can fail. This prevents an error
> elsewhere in the probe function from causing the file to leak. Do the
> same for the call to of_platform_populate().
>
> Finally, checkpatch suggests an octal literal for the file permissions.
>
> Fixes: 4af34b572a85 ("drivers: soc: sunxi: Introduce SoC driver to map SRAMs")
> Fixes: 5828729bebbb ("soc: sunxi: export a regmap for EMAC clock reg on A64")
> Reviewed-by: Jernej Skrabec <[email protected]>
> Signed-off-by: Samuel Holland <[email protected]>
Tested-by: Heiko Stuebner <[email protected]>
but one thing below
> ---
>
> (no changes since v1)
>
> drivers/soc/sunxi/sunxi_sram.c | 13 +++++--------
> 1 file changed, 5 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/soc/sunxi/sunxi_sram.c b/drivers/soc/sunxi/sunxi_sram.c
> index a858a37fcdd4..52d07bed7664 100644
> --- a/drivers/soc/sunxi/sunxi_sram.c
> +++ b/drivers/soc/sunxi/sunxi_sram.c
> @@ -332,9 +332,9 @@ static struct regmap_config sunxi_sram_emac_clock_regmap = {
>
> static int __init sunxi_sram_probe(struct platform_device *pdev)
> {
> - struct dentry *d;
> struct regmap *emac_clock;
> const struct sunxi_sramc_variant *variant;
> + struct device *dev = &pdev->dev;
>
> sram_dev = &pdev->dev;
>
> @@ -346,13 +346,6 @@ static int __init sunxi_sram_probe(struct platform_device *pdev)
> if (IS_ERR(base))
> return PTR_ERR(base);
>
> - of_platform_populate(pdev->dev.of_node, NULL, NULL, &pdev->dev);
> -
> - d = debugfs_create_file("sram", S_IRUGO, NULL, NULL,
> - &sunxi_sram_fops);
> - if (!d)
> - return -ENOMEM;
> -
> if (variant->num_emac_clocks > 0) {
> emac_clock = devm_regmap_init_mmio(&pdev->dev, base,
> &sunxi_sram_emac_clock_regmap);
> @@ -361,6 +354,10 @@ static int __init sunxi_sram_probe(struct platform_device *pdev)
> return PTR_ERR(emac_clock);
> }
>
> + of_platform_populate(dev->of_node, NULL, NULL, dev);
hmm, of_platform_populate() can actually fail [0] it just looks a bit like
sunxi driver seem to ignore that by {chance, design?} [1] .
So I guess this might want to have handling for probably unlikely
possible errors instead?
Heiko
[0] https://elixir.bootlin.com/linux/latest/source/drivers/of/platform.c#L463
[1] https://elixir.bootlin.com/linux/latest/source/drivers/bus/sun50i-de2.c#L22
> +
> + debugfs_create_file("sram", 0444, NULL, NULL, &sunxi_sram_fops);
> +
> return 0;
> }
>
>
Am Montag, 15. August 2022, 06:12:47 CEST schrieb Samuel Holland:
> D1 has a single EMAC and some LDOs that need to be exported.
>
> Acked-by: Jernej Skrabec <[email protected]>
> Signed-off-by: Samuel Holland <[email protected]>
Reviewed-by: Heiko Stuebner <[email protected]>
Tested-by: Heiko Stuebner <[email protected]>
On 8/15/22 9:06 AM, Heiko Stübner wrote:
> Am Montag, 15. August 2022, 06:12:42 CEST schrieb Samuel Holland:
>> Errors from debugfs are intended to be non-fatal, and should not prevent
>> the driver from probing.
>>
>> Since debugfs file creation is treated as infallible, move it below the
>> parts of the probe function that can fail. This prevents an error
>> elsewhere in the probe function from causing the file to leak. Do the
>> same for the call to of_platform_populate().
>>
>> Finally, checkpatch suggests an octal literal for the file permissions.
>>
>> Fixes: 4af34b572a85 ("drivers: soc: sunxi: Introduce SoC driver to map SRAMs")
>> Fixes: 5828729bebbb ("soc: sunxi: export a regmap for EMAC clock reg on A64")
>> Reviewed-by: Jernej Skrabec <[email protected]>
>> Signed-off-by: Samuel Holland <[email protected]>
>
> Tested-by: Heiko Stuebner <[email protected]>
>
> but one thing below
>
>> ---
>>
>> (no changes since v1)
>>
>> drivers/soc/sunxi/sunxi_sram.c | 13 +++++--------
>> 1 file changed, 5 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/soc/sunxi/sunxi_sram.c b/drivers/soc/sunxi/sunxi_sram.c
>> index a858a37fcdd4..52d07bed7664 100644
>> --- a/drivers/soc/sunxi/sunxi_sram.c
>> +++ b/drivers/soc/sunxi/sunxi_sram.c
>> @@ -332,9 +332,9 @@ static struct regmap_config sunxi_sram_emac_clock_regmap = {
>>
>> static int __init sunxi_sram_probe(struct platform_device *pdev)
>> {
>> - struct dentry *d;
>> struct regmap *emac_clock;
>> const struct sunxi_sramc_variant *variant;
>> + struct device *dev = &pdev->dev;
>>
>> sram_dev = &pdev->dev;
>>
>> @@ -346,13 +346,6 @@ static int __init sunxi_sram_probe(struct platform_device *pdev)
>> if (IS_ERR(base))
>> return PTR_ERR(base);
>>
>> - of_platform_populate(pdev->dev.of_node, NULL, NULL, &pdev->dev);
>> -
>> - d = debugfs_create_file("sram", S_IRUGO, NULL, NULL,
>> - &sunxi_sram_fops);
>> - if (!d)
>> - return -ENOMEM;
>> -
>> if (variant->num_emac_clocks > 0) {
>> emac_clock = devm_regmap_init_mmio(&pdev->dev, base,
>> &sunxi_sram_emac_clock_regmap);
>> @@ -361,6 +354,10 @@ static int __init sunxi_sram_probe(struct platform_device *pdev)
>> return PTR_ERR(emac_clock);
>> }
>>
>> + of_platform_populate(dev->of_node, NULL, NULL, dev);
>
> hmm, of_platform_populate() can actually fail [0] it just looks a bit like
> sunxi driver seem to ignore that by {chance, design?} [1] .
>
> So I guess this might want to have handling for probably unlikely
> possible errors instead?
Strictly speaking, neither this driver nor the DE2 bus driver depend on any of
the child nodes having a platform device present or a driver attached. So
failing to populate the child devices should not necessarily prevent this driver
from probing. Possibly it deserves a dev_warn(), but...
I don't think of_platform_populate() can actually fail when passed a valid node.
of_platform_bus_create() calls itself recursively, but otherwise always returns 0.
Regards,
Samuel
> Heiko
>
> [0] https://elixir.bootlin.com/linux/latest/source/drivers/of/platform.c#L463
> [1] https://elixir.bootlin.com/linux/latest/source/drivers/bus/sun50i-de2.c#L22
>> +
>> + debugfs_create_file("sram", 0444, NULL, NULL, &sunxi_sram_fops);
>> +
>> return 0;
>> }
On 15/08/2022 07:12, Samuel Holland wrote:
> Use enumerations where appropriate to combine "const" choices and
> deduplicate fallback compatible strings.
>
> Suggested-by: Krzysztof Kozlowski <[email protected]>
> Signed-off-by: Samuel Holland <[email protected]>
> ---
>
> Changes in v2:
Reviewed-by: Krzysztof Kozlowski <[email protected]>
Best regards,
Krzysztof
On 15/08/2022 07:12, Samuel Holland wrote:
> D1 needs to export a register for managing some LDO regulators, so it
> needs a unique compatible.
>
> Signed-off-by: Samuel Holland <[email protected]>
Acked-by: Krzysztof Kozlowski <[email protected]>
Best regards,
Krzysztof
Dne ponedeljek, 15. avgust 2022 ob 06:12:37 CEST je Samuel Holland napisal(a):
> This series cleans up a few issues in the system controller driver, and
> then expands the exported regmap to support one of the pairs of LDOs
> built in to the D1 SoC.
>
> Eventually, we will need to update the SRAM region claiming API so
> ownership can be swapped back and forth by consumer drivers. This will
> be necessary for uploading firmware to the R329/D1 DSPs, but it is not
> needed for initial bringup.
>
> Changes in v2:
> - New patch to first clean up the binding
>
> Samuel Holland (10):
> dt-bindings: sram: sunxi-sram: Clean up the compatible lists
> dt-bindings: sram: sunxi-sram: Add D1 compatible string
> soc: sunxi: sram: Actually claim SRAM regions
> soc: sunxi: sram: Prevent the driver from being unbound
> soc: sunxi: sram: Fix probe function ordering issues
> soc: sunxi: sram: Fix debugfs info for A64 SRAM C
> soc: sunxi: sram: Return void from the release function
> soc: sunxi: sram: Save a pointer to the OF match data
> soc: sunxi: sram: Export the LDO control register
> soc: sunxi: sram: Add support for the D1 system control
Applied all, thanks!
Best regards,
Jernej
>
> .../allwinner,sun4i-a10-system-control.yaml | 82 ++++++++-----------
> drivers/soc/sunxi/sunxi_sram.c | 74 +++++++++--------
> include/linux/soc/sunxi/sunxi_sram.h | 2 +-
> 3 files changed, 73 insertions(+), 85 deletions(-)
>
> --
> 2.35.1