2019-09-18 11:54:44

by Gareth Williams

[permalink] [raw]
Subject: [PATCH v2 0/4] spi: dw: Add basic runtime PM support

The Renesas RZ/N1 SPI Controller is based on the Synopsys DW SSI. This
series enables power mode in the driver so the clock domain will enable
the bus clock, adds the compatible string and updates the associated bindings
documentation.

v2:
- Note that pclk should be renamed when using a clock domain in the
bindings documentation.
- Set spi_controller.auto_runtime_pm instead of using
pm_runtime_get_sync.
- Added pm_runtime_disable calls to dw_spi_remove_host and the error
condition of dw_spi_add_host.

Gareth Williams (1):
dt-bindings: snps,dw-apb-ssi: Add optional clock domain information

Phil Edworthy (3):
dt: spi: Add Renesas RZ/N1 binding documentation
spi: dw: Add basic runtime PM support
spi: dw: Add compatible string for Renesas RZ/N1 SPI Controller

Documentation/devicetree/bindings/spi/renesas,rzn1-spi.txt | 11 +++++++++++
Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt | 3 ++-
drivers/spi/spi-dw-mmio.c | 1 +
drivers/spi/spi-dw.c | 8 ++++++++
4 files changed, 22 insertions(+), 1 deletion(-)
create mode 100644 Documentation/devicetree/bindings/spi/renesas,rzn1-spi.txt

--
2.7.4


2019-09-18 11:54:57

by Gareth Williams

[permalink] [raw]
Subject: [PATCH v2 2/4] dt-bindings: snps,dw-apb-ssi: Add optional clock domain information

Note in the bindings documentation that pclk should be renamed if a clock
domain is used to enable the optional bus clock.

Signed-off-by: Gareth Williams <[email protected]>
---
v2: Introduced this patch.
---
Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
index f54c8c3..3ed08ee 100644
--- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
+++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
@@ -16,7 +16,8 @@ Required properties:
Optional properties:
- clock-names : Contains the names of the clocks:
"ssi_clk", for the core clock used to generate the external SPI clock.
- "pclk", the interface clock, required for register access.
+ "pclk", the interface clock, required for register access. If a clock domain
+ used to enable this clock then it should be named "pclk_clkdomain".
- cs-gpios : Specifies the gpio pins to be used for chipselects.
- num-cs : The number of chipselects. If omitted, this will default to 4.
- reg-io-width : The I/O register width (in bytes) implemented by this
--
2.7.4

2019-09-18 11:55:34

by Gareth Williams

[permalink] [raw]
Subject: [PATCH v2 4/4] spi: dw: Add compatible string for Renesas RZ/N1 SPI Controller

From: Phil Edworthy <[email protected]>

The Renesas RZ/N1 SPI Controller is based on the Synopsys DW SSI, but has
additional registers for software CS control and DMA. This patch does not
address the changes required for DMA support, it simply adds the compatible
string. The CS registers are not needed as Linux can use gpios for the CS
signals.

Signed-off-by: Gareth Williams <[email protected]>
Signed-off-by: Phil Edworthy <[email protected]>
---
v2: no changes
---
drivers/spi/spi-dw-mmio.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/spi/spi-dw-mmio.c b/drivers/spi/spi-dw-mmio.c
index edb3cf6..3640b01 100644
--- a/drivers/spi/spi-dw-mmio.c
+++ b/drivers/spi/spi-dw-mmio.c
@@ -225,6 +225,7 @@ static const struct of_device_id dw_spi_mmio_of_match[] = {
{ .compatible = "mscc,ocelot-spi", .data = dw_spi_mscc_ocelot_init},
{ .compatible = "mscc,jaguar2-spi", .data = dw_spi_mscc_jaguar2_init},
{ .compatible = "amazon,alpine-dw-apb-ssi", .data = dw_spi_alpine_init},
+ { .compatible = "renesas,rzn1-spi", },
{ /* end of table */}
};
MODULE_DEVICE_TABLE(of, dw_spi_mmio_of_match);
--
2.7.4

2019-09-18 11:56:55

by Gareth Williams

[permalink] [raw]
Subject: [PATCH v2 3/4] spi: dw: Add basic runtime PM support

From: Phil Edworthy <[email protected]>

Enable runtime PM so that the clock used to access the registers in the
peripheral is turned on using a clock domain.

Signed-off-by: Phil Edworthy <[email protected]>
Signed-off-by: Gareth Williams <[email protected]>
---
v2:
- set spi_controller.auto_runtime_pm instead of using
pm_runtime_get_sync.
- Added pm_runtime_disable calls to dw_spi_remove_host and the error
condition of dw_spi_add_host.
---
drivers/spi/spi-dw.c | 8 ++++++++
1 file changed, 8 insertions(+)

diff --git a/drivers/spi/spi-dw.c b/drivers/spi/spi-dw.c
index 9a49e07..54ed6eb 100644
--- a/drivers/spi/spi-dw.c
+++ b/drivers/spi/spi-dw.c
@@ -10,6 +10,7 @@
#include <linux/module.h>
#include <linux/highmem.h>
#include <linux/delay.h>
+#include <linux/pm_runtime.h>
#include <linux/slab.h>
#include <linux/spi/spi.h>

@@ -493,10 +494,13 @@ int dw_spi_add_host(struct device *dev, struct dw_spi *dws)
master->dev.of_node = dev->of_node;
master->dev.fwnode = dev->fwnode;
master->flags = SPI_MASTER_GPIO_SS;
+ master->auto_runtime_pm = true;

if (dws->set_cs)
master->set_cs = dws->set_cs;

+ pm_runtime_enable(dev);
+
/* Basic HW init */
spi_hw_init(dev, dws);

@@ -525,6 +529,7 @@ int dw_spi_add_host(struct device *dev, struct dw_spi *dws)
spi_enable_chip(dws, 0);
free_irq(dws->irq, master);
err_free_master:
+ pm_runtime_disable(dev);
spi_controller_put(master);
return ret;
}
@@ -539,6 +544,9 @@ void dw_spi_remove_host(struct dw_spi *dws)

spi_shutdown_chip(dws);

+ if (dws->master)
+ pm_runtime_disable(&dws->master->dev);
+
free_irq(dws->irq, dws->master);
}
EXPORT_SYMBOL_GPL(dw_spi_remove_host);
--
2.7.4

2019-09-18 12:52:21

by Gareth Williams

[permalink] [raw]
Subject: [PATCH v2 1/4] dt: spi: Add Renesas RZ/N1 binding documentation

From: Phil Edworthy <[email protected]>

The Renesas RZ/N1 SPI Controller is based on the Synopsys DW SSI, but has
additional registers for software CS control and DMA. This patch does not
address the changes required for DMA support, it simply adds the compatible
string. The CS functionality is not very useful and also not needed as
Linux can use gpios for the CS signals.

Add a compatible string to handle any unforeseen issues that may arise, and
pave the way for DMA support.

Signed-off-by: Gareth Williams <[email protected]>
Signed-off-by: Phil Edworthy <[email protected]>
---
Note: All the other manufacturers detail their compatible strings in
snps,dw-apb-ssi.txt. I think it makes sense for rzn1 to be in it's own file
due to the changes made to the peripheral for DMA support.

v2:
- No changes.
---
Documentation/devicetree/bindings/spi/renesas,rzn1-spi.txt | 11 +++++++++++
1 file changed, 11 insertions(+)
create mode 100644 Documentation/devicetree/bindings/spi/renesas,rzn1-spi.txt

diff --git a/Documentation/devicetree/bindings/spi/renesas,rzn1-spi.txt b/Documentation/devicetree/bindings/spi/renesas,rzn1-spi.txt
new file mode 100644
index 0000000..fb1a672
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/renesas,rzn1-spi.txt
@@ -0,0 +1,11 @@
+Renesas RZ/N1 SPI Controller
+
+This controller is based on the Synopsys DW Synchronous Serial Interface and
+inherits all properties defined in snps,dw-apb-ssi.txt except for the
+compatible property.
+
+Required properties:
+- compatible : The device specific string followed by the generic RZ/N1 string.
+ Therefore it must be one of:
+ "renesas,r9a06g032-spi", "renesas,rzn1-spi"
+ "renesas,r9a06g033-spi", "renesas,rzn1-spi"
--
2.7.4

2019-09-19 13:57:00

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH v2 0/4] spi: dw: Add basic runtime PM support

On Wed, Sep 18, 2019 at 09:04:32AM +0100, Gareth Williams wrote:

> Gareth Williams (1):
> dt-bindings: snps,dw-apb-ssi: Add optional clock domain information
>
> Phil Edworthy (3):
> dt: spi: Add Renesas RZ/N1 binding documentation

Please use subject lines matching the style for the subsystem. This
makes it easier for people to identify relevant patches. This isn't
even consistent within the series :(


Attachments:
(No filename) (425.00 B)
signature.asc (499.00 B)
Download all attachments

2019-09-19 21:31:46

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH v2 0/4] spi: dw: Add basic runtime PM support

On Thu, Sep 19, 2019 at 03:14:54PM +0000, Gareth Williams wrote:
> On Wed, Sep 19, 2019 at 14:31:32AM +0100, Mark Brown wrote:

> > Please use subject lines matching the style for the subsystem. This makes it
> > easier for people to identify relevant patches. This isn't even consistent
> > within the series :(

> Sorry about that, I will correct the subject lines for V3.

Don't worry about it unless you need to send a v3 for some other reason.

> Is there a set convention for the subsystem I should follow in future?
> Or should I follow the style of the individual files I work on?

Following the style for the file/directory is generally a good guide,
for SPI I tend to prefer spi: but I just moan about it rather than block
anything for it (unless I do end up missing the patch in my inbox).


Attachments:
(No filename) (820.00 B)
signature.asc (499.00 B)
Download all attachments

2019-09-19 23:42:07

by Gareth Williams

[permalink] [raw]
Subject: RE: [PATCH v2 0/4] spi: dw: Add basic runtime PM support

Hi Mark,

On Wed, Sep 19, 2019 at 14:31:32AM +0100, Mark Brown wrote:

> On Wed, Sep 18, 2019 at 09:04:32AM +0100, Gareth Williams wrote:
>
> > Gareth Williams (1):
> > dt-bindings: snps,dw-apb-ssi: Add optional clock domain information
> >
> > Phil Edworthy (3):
> > dt: spi: Add Renesas RZ/N1 binding documentation
>
> Please use subject lines matching the style for the subsystem. This makes it
> easier for people to identify relevant patches. This isn't even consistent
> within the series :(
Sorry about that, I will correct the subject lines for V3.
Is there a set convention for the subsystem I should follow in future?
Or should I follow the style of the individual files I work on?

Kind Regards,

Gareth

2019-10-01 11:42:26

by Mark Brown

[permalink] [raw]
Subject: Applied "spi: dw: Add compatible string for Renesas RZ/N1 SPI Controller" to the spi tree

The patch

spi: dw: Add compatible string for Renesas RZ/N1 SPI Controller

has been applied to the spi tree at

https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-5.5

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

From 3ade3a37bdd77319dd7865228402cf1669c9b678 Mon Sep 17 00:00:00 2001
From: Phil Edworthy <[email protected]>
Date: Wed, 18 Sep 2019 09:04:36 +0100
Subject: [PATCH] spi: dw: Add compatible string for Renesas RZ/N1 SPI
Controller

The Renesas RZ/N1 SPI Controller is based on the Synopsys DW SSI, but has
additional registers for software CS control and DMA. This patch does not
address the changes required for DMA support, it simply adds the compatible
string. The CS registers are not needed as Linux can use gpios for the CS
signals.

Signed-off-by: Gareth Williams <[email protected]>
Signed-off-by: Phil Edworthy <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Mark Brown <[email protected]>
---
drivers/spi/spi-dw-mmio.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/spi/spi-dw-mmio.c b/drivers/spi/spi-dw-mmio.c
index bd46fca3f094..b5ce8bd58d9e 100644
--- a/drivers/spi/spi-dw-mmio.c
+++ b/drivers/spi/spi-dw-mmio.c
@@ -223,6 +223,7 @@ static const struct of_device_id dw_spi_mmio_of_match[] = {
{ .compatible = "mscc,ocelot-spi", .data = dw_spi_mscc_ocelot_init},
{ .compatible = "mscc,jaguar2-spi", .data = dw_spi_mscc_jaguar2_init},
{ .compatible = "amazon,alpine-dw-apb-ssi", .data = dw_spi_alpine_init},
+ { .compatible = "renesas,rzn1-spi", },
{ /* end of table */}
};
MODULE_DEVICE_TABLE(of, dw_spi_mmio_of_match);
--
2.20.1

2019-10-01 11:42:39

by Mark Brown

[permalink] [raw]
Subject: Applied "spi: dw: Add basic runtime PM support" to the spi tree

The patch

spi: dw: Add basic runtime PM support

has been applied to the spi tree at

https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-5.5

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

From 1e695983251029dc0b0fc516290077539df400ff Mon Sep 17 00:00:00 2001
From: Phil Edworthy <[email protected]>
Date: Wed, 18 Sep 2019 09:04:35 +0100
Subject: [PATCH] spi: dw: Add basic runtime PM support

Enable runtime PM so that the clock used to access the registers in the
peripheral is turned on using a clock domain.

Signed-off-by: Phil Edworthy <[email protected]>
Signed-off-by: Gareth Williams <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Mark Brown <[email protected]>
---
drivers/spi/spi-dw.c | 8 ++++++++
1 file changed, 8 insertions(+)

diff --git a/drivers/spi/spi-dw.c b/drivers/spi/spi-dw.c
index 9a49e073e8b7..54ed6eb3e252 100644
--- a/drivers/spi/spi-dw.c
+++ b/drivers/spi/spi-dw.c
@@ -10,6 +10,7 @@
#include <linux/module.h>
#include <linux/highmem.h>
#include <linux/delay.h>
+#include <linux/pm_runtime.h>
#include <linux/slab.h>
#include <linux/spi/spi.h>

@@ -493,10 +494,13 @@ int dw_spi_add_host(struct device *dev, struct dw_spi *dws)
master->dev.of_node = dev->of_node;
master->dev.fwnode = dev->fwnode;
master->flags = SPI_MASTER_GPIO_SS;
+ master->auto_runtime_pm = true;

if (dws->set_cs)
master->set_cs = dws->set_cs;

+ pm_runtime_enable(dev);
+
/* Basic HW init */
spi_hw_init(dev, dws);

@@ -525,6 +529,7 @@ int dw_spi_add_host(struct device *dev, struct dw_spi *dws)
spi_enable_chip(dws, 0);
free_irq(dws->irq, master);
err_free_master:
+ pm_runtime_disable(dev);
spi_controller_put(master);
return ret;
}
@@ -539,6 +544,9 @@ void dw_spi_remove_host(struct dw_spi *dws)

spi_shutdown_chip(dws);

+ if (dws->master)
+ pm_runtime_disable(&dws->master->dev);
+
free_irq(dws->irq, dws->master);
}
EXPORT_SYMBOL_GPL(dw_spi_remove_host);
--
2.20.1

2019-10-01 11:43:05

by Mark Brown

[permalink] [raw]
Subject: Applied "dt: spi: Add Renesas RZ/N1 binding documentation" to the spi tree

The patch

dt: spi: Add Renesas RZ/N1 binding documentation

has been applied to the spi tree at

https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-5.5

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

From da182a61fce01dbd7c4a78c68a34af110f00e36f Mon Sep 17 00:00:00 2001
From: Phil Edworthy <[email protected]>
Date: Wed, 18 Sep 2019 09:04:33 +0100
Subject: [PATCH] dt: spi: Add Renesas RZ/N1 binding documentation

The Renesas RZ/N1 SPI Controller is based on the Synopsys DW SSI, but has
additional registers for software CS control and DMA. This patch does not
address the changes required for DMA support, it simply adds the compatible
string. The CS functionality is not very useful and also not needed as
Linux can use gpios for the CS signals.

Add a compatible string to handle any unforeseen issues that may arise, and
pave the way for DMA support.

Signed-off-by: Gareth Williams <[email protected]>
Signed-off-by: Phil Edworthy <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Mark Brown <[email protected]>
---
.../devicetree/bindings/spi/renesas,rzn1-spi.txt | 11 +++++++++++
1 file changed, 11 insertions(+)
create mode 100644 Documentation/devicetree/bindings/spi/renesas,rzn1-spi.txt

diff --git a/Documentation/devicetree/bindings/spi/renesas,rzn1-spi.txt b/Documentation/devicetree/bindings/spi/renesas,rzn1-spi.txt
new file mode 100644
index 000000000000..fb1a6728638d
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/renesas,rzn1-spi.txt
@@ -0,0 +1,11 @@
+Renesas RZ/N1 SPI Controller
+
+This controller is based on the Synopsys DW Synchronous Serial Interface and
+inherits all properties defined in snps,dw-apb-ssi.txt except for the
+compatible property.
+
+Required properties:
+- compatible : The device specific string followed by the generic RZ/N1 string.
+ Therefore it must be one of:
+ "renesas,r9a06g032-spi", "renesas,rzn1-spi"
+ "renesas,r9a06g033-spi", "renesas,rzn1-spi"
--
2.20.1

2019-10-01 11:45:18

by Mark Brown

[permalink] [raw]
Subject: Applied "dt-bindings: snps,dw-apb-ssi: Add optional clock domain information" to the spi tree

The patch

dt-bindings: snps,dw-apb-ssi: Add optional clock domain information

has been applied to the spi tree at

https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-5.5

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

From 47cf13bc763c891c6192184c5e5aa8c1b331b2ff Mon Sep 17 00:00:00 2001
From: Gareth Williams <[email protected]>
Date: Wed, 18 Sep 2019 09:04:34 +0100
Subject: [PATCH] dt-bindings: snps,dw-apb-ssi: Add optional clock domain
information

Note in the bindings documentation that pclk should be renamed if a clock
domain is used to enable the optional bus clock.

Signed-off-by: Gareth Williams <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Mark Brown <[email protected]>
---
Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
index f54c8c36395e..3ed08ee9feba 100644
--- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
+++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
@@ -16,7 +16,8 @@ Required properties:
Optional properties:
- clock-names : Contains the names of the clocks:
"ssi_clk", for the core clock used to generate the external SPI clock.
- "pclk", the interface clock, required for register access.
+ "pclk", the interface clock, required for register access. If a clock domain
+ used to enable this clock then it should be named "pclk_clkdomain".
- cs-gpios : Specifies the gpio pins to be used for chipselects.
- num-cs : The number of chipselects. If omitted, this will default to 4.
- reg-io-width : The I/O register width (in bytes) implemented by this
--
2.20.1

2019-10-01 11:47:31

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v2 1/4] dt: spi: Add Renesas RZ/N1 binding documentation

On Wed, 18 Sep 2019 09:04:33 +0100, Gareth Williams wrote:
> From: Phil Edworthy <[email protected]>
>
> The Renesas RZ/N1 SPI Controller is based on the Synopsys DW SSI, but has
> additional registers for software CS control and DMA. This patch does not
> address the changes required for DMA support, it simply adds the compatible
> string. The CS functionality is not very useful and also not needed as
> Linux can use gpios for the CS signals.
>
> Add a compatible string to handle any unforeseen issues that may arise, and
> pave the way for DMA support.
>
> Signed-off-by: Gareth Williams <[email protected]>
> Signed-off-by: Phil Edworthy <[email protected]>
> ---
> Note: All the other manufacturers detail their compatible strings in
> snps,dw-apb-ssi.txt. I think it makes sense for rzn1 to be in it's own file
> due to the changes made to the peripheral for DMA support.
>
> v2:
> - No changes.
> ---
> Documentation/devicetree/bindings/spi/renesas,rzn1-spi.txt | 11 +++++++++++
> 1 file changed, 11 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/spi/renesas,rzn1-spi.txt
>

Reviewed-by: Rob Herring <[email protected]>

2019-10-01 12:03:55

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v2 2/4] dt-bindings: snps,dw-apb-ssi: Add optional clock domain information

On Wed, Sep 18, 2019 at 09:04:34AM +0100, Gareth Williams wrote:
> Note in the bindings documentation that pclk should be renamed if a clock
> domain is used to enable the optional bus clock.
>
> Signed-off-by: Gareth Williams <[email protected]>
> ---
> v2: Introduced this patch.
> ---
> Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
> index f54c8c3..3ed08ee 100644
> --- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
> +++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
> @@ -16,7 +16,8 @@ Required properties:
> Optional properties:
> - clock-names : Contains the names of the clocks:
> "ssi_clk", for the core clock used to generate the external SPI clock.
> - "pclk", the interface clock, required for register access.
> + "pclk", the interface clock, required for register access. If a clock domain
> + used to enable this clock then it should be named "pclk_clkdomain".

What's a clock domain?

Unless this is a h/w difference in the IP block, then this change
doesn't make sense.

> - cs-gpios : Specifies the gpio pins to be used for chipselects.
> - num-cs : The number of chipselects. If omitted, this will default to 4.
> - reg-io-width : The I/O register width (in bytes) implemented by this
> --
> 2.7.4
>

2019-10-01 13:53:07

by Gareth Williams

[permalink] [raw]
Subject: RE: [PATCH v2 2/4] dt-bindings: snps,dw-apb-ssi: Add optional clock domain information

Hi Rob,

On Tue, Oct 01, 2019 at 13:02:34AM +0100, Rob Herring wrote:
> On Wed, Sep 18, 2019 at 09:04:34AM +0100, Gareth Williams wrote:
> > Note in the bindings documentation that pclk should be renamed if a
> > clock domain is used to enable the optional bus clock.
> >
> > Signed-off-by: Gareth Williams <[email protected]>
> > ---
> > v2: Introduced this patch.
> > ---
> > Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
> > b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
> > index f54c8c3..3ed08ee 100644
> > --- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
> > +++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
> > @@ -16,7 +16,8 @@ Required properties:
> > Optional properties:
> > - clock-names : Contains the names of the clocks:
> > "ssi_clk", for the core clock used to generate the external SPI clock.
> > - "pclk", the interface clock, required for register access.
> > + "pclk", the interface clock, required for register access. If a clock domain
> > + used to enable this clock then it should be named "pclk_clkdomain".
>
> What's a clock domain?
>
> Unless this is a h/w difference in the IP block, then this change doesn't make
> sense.
This is a reference to the use of clock domains that are implemented through
generic power domains. The domain is implemented in
drivers/clk/renesas/r9a06g032-clocks.c and general details of clock domains
can be found at
https://elinux.org/images/1/14/Last_One_Out%2C_Turn_Off_The_Lights.pdf

>
> > - cs-gpios : Specifies the gpio pins to be used for chipselects.
> > - num-cs : The number of chipselects. If omitted, this will default to 4.
> > - reg-io-width : The I/O register width (in bytes) implemented by
> > this
> > --
> > 2.7.4
> >

2019-10-01 14:54:27

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH v2 2/4] dt-bindings: snps,dw-apb-ssi: Add optional clock domain information

Hi Gareth,

On Tue, Oct 1, 2019 at 3:50 PM Gareth Williams
<[email protected]> wrote:
> On Tue, Oct 01, 2019 at 13:02:34AM +0100, Rob Herring wrote:
> > On Wed, Sep 18, 2019 at 09:04:34AM +0100, Gareth Williams wrote:
> > > Note in the bindings documentation that pclk should be renamed if a
> > > clock domain is used to enable the optional bus clock.
> > >
> > > Signed-off-by: Gareth Williams <[email protected]>
> > > ---
> > > v2: Introduced this patch.
> > > ---
> > > Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt | 3 ++-
> > > 1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
> > > b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
> > > index f54c8c3..3ed08ee 100644
> > > --- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
> > > +++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
> > > @@ -16,7 +16,8 @@ Required properties:
> > > Optional properties:
> > > - clock-names : Contains the names of the clocks:
> > > "ssi_clk", for the core clock used to generate the external SPI clock.
> > > - "pclk", the interface clock, required for register access.
> > > + "pclk", the interface clock, required for register access. If a clock domain
> > > + used to enable this clock then it should be named "pclk_clkdomain".
> >
> > What's a clock domain?
> >
> > Unless this is a h/w difference in the IP block, then this change doesn't make
> > sense.
> This is a reference to the use of clock domains that are implemented through
> generic power domains. The domain is implemented in
> drivers/clk/renesas/r9a06g032-clocks.c and general details of clock domains
> can be found at
> https://elinux.org/images/1/14/Last_One_Out%2C_Turn_Off_The_Lights.pdf

Rob is right: the clock domain is an SoC integration detail, not specific to
the snps,dw-apb-ssi block.
Remember, DT describes hardware, not implementation details.

So the Linux snps,dw-apb-ssi driver should take care of it.

Which brings us back to an old discussion topic: power-domains properties
describe integration, and thus should be documented at a higher level than
in individual binding documents, just like e.g. interrupt-parent.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds