* Re: [PATCH v6 19/23] dt-bindings: crypto: meson: support new SoC's
From: Conor Dooley @ 2024-03-26 18:04 UTC (permalink / raw)
To: Alexey Romanov
Cc: neil.armstrong, clabbe, herbert, davem, robh+dt,
krzysztof.kozlowski+dt, conor+dt, khilman, jbrunet,
martin.blumenstingl, vadim.fedorenko, linux-crypto, linux-amlogic,
linux-kernel, devicetree, linux-arm-kernel, kernel
In-Reply-To: <20240326153219.2915080-20-avromanov@salutedevices.com>
[-- Attachment #1.1: Type: text/plain, Size: 1543 bytes --]
On Tue, Mar 26, 2024 at 06:32:15PM +0300, Alexey Romanov wrote:
> Now crypto module available at G12A/G12B/S4/A1/SM1/AXG.
>
> 1. Add new compatibles:
> - amlogic,g12a-crypto
> - amlogic,axg-crypto
> - amlogic,a1-crypto
> - amlogic,s4-crypto (uses a1-crypto as fallback)
>
> 2. Add power-domains in schema.
>
> Signed-off-by: Alexey Romanov <avromanov@salutedevices.com>
> ---
> .../bindings/crypto/amlogic,gxl-crypto.yaml | 15 +++++++++++++--
> 1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/crypto/amlogic,gxl-crypto.yaml b/Documentation/devicetree/bindings/crypto/amlogic,gxl-crypto.yaml
> index d3af7b4d5f39..c92edde314aa 100644
> --- a/Documentation/devicetree/bindings/crypto/amlogic,gxl-crypto.yaml
> +++ b/Documentation/devicetree/bindings/crypto/amlogic,gxl-crypto.yaml
> @@ -11,8 +11,16 @@ maintainers:
>
> properties:
> compatible:
> - items:
> - - const: amlogic,gxl-crypto
> + oneOf:
> + - items:
> + - enum:
> + - amlogic,s4-crypto
> + - const: amlogic,a1-crypto
> + - enum:
> + - amlogic,gxl-crypto
> + - amlogic,axg-crypto
> + - amlogic,g12a-crypto
> + - amlogic,a1-crypto
>
> reg:
> maxItems: 1
> @@ -21,6 +29,9 @@ properties:
> items:
> - description: Interrupt for flow 0
>
> + power-domains:
> + maxItems: 1
Is power-domains valid for the devices that existed prior to your patch?
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v6 17/23] dt-bindings: crypto: meson: remove clk and second interrupt line for GXL
From: Conor Dooley @ 2024-03-26 18:09 UTC (permalink / raw)
To: Alexey Romanov
Cc: neil.armstrong, clabbe, herbert, davem, robh+dt,
krzysztof.kozlowski+dt, conor+dt, khilman, jbrunet,
martin.blumenstingl, vadim.fedorenko, linux-crypto, linux-amlogic,
linux-kernel, devicetree, linux-arm-kernel, kernel
In-Reply-To: <20240326153219.2915080-18-avromanov@salutedevices.com>
[-- Attachment #1.1: Type: text/plain, Size: 1864 bytes --]
On Tue, Mar 26, 2024 at 06:32:13PM +0300, Alexey Romanov wrote:
> GXL crypto IP uses DMA engine, which doesn't require clk input
> and second interrupt line.
>
> Fixes: c4a0457eb858 ("ARM64: dts: amlogic: adds crypto hardware node")
How does this fix a commit that only modifies a dts? The commit cited
here should be a dt-bindings commit.
However, your commit message says "require" but you do more than remove
these as required, you remove them entirely. What am I missing?
Thanks,
Conor.
> Signed-off-by: Alexey Romanov <avromanov@salutedevices.com>
> ---
> .../bindings/crypto/amlogic,gxl-crypto.yaml | 13 +------------
> 1 file changed, 1 insertion(+), 12 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/crypto/amlogic,gxl-crypto.yaml b/Documentation/devicetree/bindings/crypto/amlogic,gxl-crypto.yaml
> index 948e11ebe4ee..d3af7b4d5f39 100644
> --- a/Documentation/devicetree/bindings/crypto/amlogic,gxl-crypto.yaml
> +++ b/Documentation/devicetree/bindings/crypto/amlogic,gxl-crypto.yaml
> @@ -20,20 +20,11 @@ properties:
> interrupts:
> items:
> - description: Interrupt for flow 0
> - - description: Interrupt for flow 1
> -
> - clocks:
> - maxItems: 1
> -
> - clock-names:
> - const: blkmv
>
> required:
> - compatible
> - reg
> - interrupts
> - - clocks
> - - clock-names
>
> additionalProperties: false
>
> @@ -46,7 +37,5 @@ examples:
> crypto: crypto-engine@c883e000 {
> compatible = "amlogic,gxl-crypto";
> reg = <0xc883e000 0x36>;
> - interrupts = <GIC_SPI 188 IRQ_TYPE_EDGE_RISING>, <GIC_SPI 189 IRQ_TYPE_EDGE_RISING>;
> - clocks = <&clkc CLKID_BLKMV>;
> - clock-names = "blkmv";
> + interrupts = <GIC_SPI 188 IRQ_TYPE_EDGE_RISING>;
> };
> --
> 2.34.1
>
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v1 02/10] spi: pxa2xx: Keep PXA*_SSP types together
From: Andy Shevchenko @ 2024-03-26 18:07 UTC (permalink / raw)
To: Andy Shevchenko, Mark Brown, linux-spi, linux-kernel,
linux-arm-kernel
Cc: Daniel Mack, Haojian Zhuang, Robert Jarzmik, Russell King
In-Reply-To: <20240326181027.1418989-1-andriy.shevchenko@linux.intel.com>
Keep the PXA*_SSP types together in enum pxa_ssp_type
for better maintenance.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
include/linux/pxa2xx_ssp.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/pxa2xx_ssp.h b/include/linux/pxa2xx_ssp.h
index cd1973e6ac4b..844a2743ca94 100644
--- a/include/linux/pxa2xx_ssp.h
+++ b/include/linux/pxa2xx_ssp.h
@@ -217,9 +217,9 @@ enum pxa_ssp_type {
PXA27x_SSP,
PXA3xx_SSP,
PXA168_SSP,
- MMP2_SSP,
PXA910_SSP,
CE4100_SSP,
+ MMP2_SSP,
MRFLD_SSP,
QUARK_X1000_SSP,
/* Keep LPSS types sorted with lpss_platforms[] */
--
2.43.0.rc1.1.gbec44491f096
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v1 03/10] spi: pxa2xx: Switch to use dev_err_probe()
From: Andy Shevchenko @ 2024-03-26 18:07 UTC (permalink / raw)
To: Andy Shevchenko, Mark Brown, linux-spi, linux-kernel,
linux-arm-kernel
Cc: Daniel Mack, Haojian Zhuang, Robert Jarzmik, Russell King
In-Reply-To: <20240326181027.1418989-1-andriy.shevchenko@linux.intel.com>
Switch to use dev_err_probe() to simplify the error path and
unify a message template.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/spi/spi-pxa2xx.c | 19 +++++++------------
1 file changed, 7 insertions(+), 12 deletions(-)
diff --git a/drivers/spi/spi-pxa2xx.c b/drivers/spi/spi-pxa2xx.c
index 2c39d3ff498e..75d208087748 100644
--- a/drivers/spi/spi-pxa2xx.c
+++ b/drivers/spi/spi-pxa2xx.c
@@ -1434,20 +1434,16 @@ static int pxa2xx_spi_probe(struct platform_device *pdev)
platform_info = dev_get_platdata(dev);
if (!platform_info) {
platform_info = pxa2xx_spi_init_pdata(pdev);
- if (IS_ERR(platform_info)) {
- dev_err(&pdev->dev, "missing platform data\n");
- return PTR_ERR(platform_info);
- }
+ if (IS_ERR(platform_info))
+ return dev_err_probe(dev, PTR_ERR(platform_info), "missing platform data\n");
}
ssp = pxa_ssp_request(pdev->id, pdev->name);
if (!ssp)
ssp = &platform_info->ssp;
- if (!ssp->mmio_base) {
- dev_err(&pdev->dev, "failed to get SSP\n");
- return -ENODEV;
- }
+ if (!ssp->mmio_base)
+ return dev_err_probe(dev, -ENODEV, "failed to get SSP\n");
if (platform_info->is_target)
controller = devm_spi_alloc_target(dev, sizeof(*drv_data));
@@ -1455,8 +1451,7 @@ static int pxa2xx_spi_probe(struct platform_device *pdev)
controller = devm_spi_alloc_host(dev, sizeof(*drv_data));
if (!controller) {
- dev_err(&pdev->dev, "cannot alloc spi_controller\n");
- status = -ENOMEM;
+ status = dev_err_probe(dev, -ENOMEM, "cannot alloc spi_controller\n");
goto out_error_controller_alloc;
}
drv_data = spi_controller_get_devdata(controller);
@@ -1510,7 +1505,7 @@ static int pxa2xx_spi_probe(struct platform_device *pdev)
status = request_irq(ssp->irq, ssp_int, IRQF_SHARED, dev_name(dev),
drv_data);
if (status < 0) {
- dev_err(&pdev->dev, "cannot get IRQ %d\n", ssp->irq);
+ dev_err_probe(dev, status, "cannot get IRQ %d\n", ssp->irq);
goto out_error_controller_alloc;
}
@@ -1626,7 +1621,7 @@ static int pxa2xx_spi_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, drv_data);
status = spi_register_controller(controller);
if (status) {
- dev_err(&pdev->dev, "problem registering SPI controller\n");
+ dev_err_probe(dev, status, "problem registering SPI controller\n");
goto out_error_pm_runtime_enabled;
}
--
2.43.0.rc1.1.gbec44491f096
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v1 01/10] spi: pxa2xx: Drop ACPI_PTR() and of_match_ptr()
From: Andy Shevchenko @ 2024-03-26 18:07 UTC (permalink / raw)
To: Andy Shevchenko, Mark Brown, linux-spi, linux-kernel,
linux-arm-kernel
Cc: Daniel Mack, Haojian Zhuang, Robert Jarzmik, Russell King
In-Reply-To: <20240326181027.1418989-1-andriy.shevchenko@linux.intel.com>
Drop rather useless use of ACPI_PTR() and of_match_ptr().
It also removes the necessity to be dependent on ACPI and
of.h inclusion.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/spi/Kconfig | 2 +-
drivers/spi/spi-pxa2xx.c | 9 +++------
2 files changed, 4 insertions(+), 7 deletions(-)
diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index bc7021da2fe9..7cf86b034083 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -817,7 +817,7 @@ config SPI_PPC4xx
config SPI_PXA2XX
tristate "PXA2xx SSP SPI master"
- depends on ARCH_PXA || ARCH_MMP || PCI || ACPI || COMPILE_TEST
+ depends on ARCH_PXA || ARCH_MMP || PCI || COMPILE_TEST
select PXA_SSP if ARCH_PXA || ARCH_MMP
help
This enables using a PXA2xx or Sodaville SSP port as a SPI master
diff --git a/drivers/spi/spi-pxa2xx.c b/drivers/spi/spi-pxa2xx.c
index 6c2a14418972..2c39d3ff498e 100644
--- a/drivers/spi/spi-pxa2xx.c
+++ b/drivers/spi/spi-pxa2xx.c
@@ -19,7 +19,6 @@
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/mod_devicetable.h>
-#include <linux/of.h>
#include <linux/platform_device.h>
#include <linux/pm_runtime.h>
#include <linux/property.h>
@@ -1730,7 +1729,6 @@ static const struct dev_pm_ops pxa2xx_spi_pm_ops = {
RUNTIME_PM_OPS(pxa2xx_spi_runtime_suspend, pxa2xx_spi_runtime_resume, NULL)
};
-#ifdef CONFIG_ACPI
static const struct acpi_device_id pxa2xx_spi_acpi_match[] = {
{ "80860F0E", LPSS_BYT_SSP },
{ "8086228E", LPSS_BSW_SSP },
@@ -1741,9 +1739,8 @@ static const struct acpi_device_id pxa2xx_spi_acpi_match[] = {
{}
};
MODULE_DEVICE_TABLE(acpi, pxa2xx_spi_acpi_match);
-#endif
-static const struct of_device_id pxa2xx_spi_of_match[] __maybe_unused = {
+static const struct of_device_id pxa2xx_spi_of_match[] = {
{ .compatible = "marvell,mmp2-ssp", .data = (void *)MMP2_SSP },
{}
};
@@ -1753,8 +1750,8 @@ static struct platform_driver driver = {
.driver = {
.name = "pxa2xx-spi",
.pm = pm_ptr(&pxa2xx_spi_pm_ops),
- .acpi_match_table = ACPI_PTR(pxa2xx_spi_acpi_match),
- .of_match_table = of_match_ptr(pxa2xx_spi_of_match),
+ .acpi_match_table = pxa2xx_spi_acpi_match,
+ .of_match_table = pxa2xx_spi_of_match,
},
.probe = pxa2xx_spi_probe,
.remove_new = pxa2xx_spi_remove,
--
2.43.0.rc1.1.gbec44491f096
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v1 00/10] spi: pxa2xx: Drop linux/spi/pxa2xx_spi.h
From: Andy Shevchenko @ 2024-03-26 18:07 UTC (permalink / raw)
To: Andy Shevchenko, Mark Brown, linux-spi, linux-kernel,
linux-arm-kernel
Cc: Daniel Mack, Haojian Zhuang, Robert Jarzmik, Russell King,
Arnd Bergmann
As Arnd suggested we may drop linux/spi/pxa2xx_spi.h as most of
its content is being used solely internally to SPI subsystem
(PXA2xx drivers). Hence this refactoring series with the additional
win of getting rid of legacy documentation.
Cc: Arnd Bergmann <arnd@arndb.de>
Andy Shevchenko (10):
spi: pxa2xx: Drop ACPI_PTR() and of_match_ptr()
spi: pxa2xx: Keep PXA*_SSP types together
spi: pxa2xx: Switch to use dev_err_probe()
spi: pxa2xx: Extract pxa2xx_spi_init_ssp() helper
spi: pxa2xx: Skip SSP initialization if it's done elsewhere
spi: pxa2xx: Allow number of chip select pins to be read from property
spi: pxa2xx: Provide num-cs for Sharp PDAs via device properties
spi: pxa2xx: Move contents of linux/spi/pxa2xx_spi.h to a local one
spi: pxa2xx: Remove outdated documentation
spi: pxa2xx: Don't use "proxy" headers
Documentation/spi/pxa2xx.rst | 208 ---------------------------------
arch/arm/mach-pxa/spitz.c | 25 ++--
drivers/spi/Kconfig | 2 +-
drivers/spi/spi-pxa2xx-dma.c | 11 +-
drivers/spi/spi-pxa2xx-pci.c | 10 +-
drivers/spi/spi-pxa2xx.c | 118 +++++++++++--------
drivers/spi/spi-pxa2xx.h | 39 ++++++-
include/linux/pxa2xx_ssp.h | 2 +-
include/linux/spi/pxa2xx_spi.h | 48 --------
9 files changed, 140 insertions(+), 323 deletions(-)
delete mode 100644 Documentation/spi/pxa2xx.rst
delete mode 100644 include/linux/spi/pxa2xx_spi.h
--
2.43.0.rc1.1.gbec44491f096
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v1 06/10] spi: pxa2xx: Allow number of chip select pins to be read from property
From: Andy Shevchenko @ 2024-03-26 18:07 UTC (permalink / raw)
To: Andy Shevchenko, Mark Brown, linux-spi, linux-kernel,
linux-arm-kernel
Cc: Daniel Mack, Haojian Zhuang, Robert Jarzmik, Russell King
In-Reply-To: <20240326181027.1418989-1-andriy.shevchenko@linux.intel.com>
In some cases the number of the chip select pins might come from
the device property. Allow driver to use it.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/spi/spi-pxa2xx.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/spi/spi-pxa2xx.c b/drivers/spi/spi-pxa2xx.c
index b01a18c89b6b..f4435c39d096 100644
--- a/drivers/spi/spi-pxa2xx.c
+++ b/drivers/spi/spi-pxa2xx.c
@@ -1358,6 +1358,7 @@ pxa2xx_spi_init_pdata(struct platform_device *pdev)
struct ssp_device *ssp = NULL;
const void *match;
bool is_lpss_priv;
+ u32 num_cs = 1;
int status;
is_lpss_priv = platform_get_resource_byname(pdev, IORESOURCE_MEM, "lpss_priv");
@@ -1394,8 +1395,11 @@ pxa2xx_spi_init_pdata(struct platform_device *pdev)
pdata->dma_filter = pxa2xx_spi_idma_filter;
}
+ /* Read number of chip select pins, if provided */
+ device_property_read_u32(dev, "num-cs", &num_cs);
+
+ pdata->num_chipselect = num_cs;
pdata->is_target = device_property_read_bool(dev, "spi-slave");
- pdata->num_chipselect = 1;
pdata->enable_dma = true;
pdata->dma_burst_size = 1;
--
2.43.0.rc1.1.gbec44491f096
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v1 04/10] spi: pxa2xx: Extract pxa2xx_spi_init_ssp() helper
From: Andy Shevchenko @ 2024-03-26 18:07 UTC (permalink / raw)
To: Andy Shevchenko, Mark Brown, linux-spi, linux-kernel,
linux-arm-kernel
Cc: Daniel Mack, Haojian Zhuang, Robert Jarzmik, Russell King
In-Reply-To: <20240326181027.1418989-1-andriy.shevchenko@linux.intel.com>
Refactor pxa2xx_spi_init_pdata() by extracting a new
pxa2xx_spi_init_ssp() helper which makes code less
twisted. It will be easier to continue refactoring for
a new coming modification.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/spi/spi-pxa2xx.c | 66 +++++++++++++++++++++++-----------------
1 file changed, 38 insertions(+), 28 deletions(-)
diff --git a/drivers/spi/spi-pxa2xx.c b/drivers/spi/spi-pxa2xx.c
index 75d208087748..e7072727c25c 100644
--- a/drivers/spi/spi-pxa2xx.c
+++ b/drivers/spi/spi-pxa2xx.c
@@ -1314,19 +1314,50 @@ static bool pxa2xx_spi_idma_filter(struct dma_chan *chan, void *param)
return param == chan->device->dev;
}
+static int
+pxa2xx_spi_init_ssp(struct platform_device *pdev, struct ssp_device *ssp, enum pxa_ssp_type type)
+{
+ struct device *dev = &pdev->dev;
+ struct resource *res;
+ int status;
+ u64 uid;
+
+ ssp->mmio_base = devm_platform_get_and_ioremap_resource(pdev, 0, &res);
+ if (IS_ERR(ssp->mmio_base))
+ return PTR_ERR(ssp->mmio_base);
+
+ ssp->phys_base = res->start;
+
+ ssp->clk = devm_clk_get(dev, NULL);
+ if (IS_ERR(ssp->clk))
+ return PTR_ERR(ssp->clk);
+
+ ssp->irq = platform_get_irq(pdev, 0);
+ if (ssp->irq < 0)
+ return ssp->irq;
+
+ ssp->type = type;
+ ssp->dev = dev;
+
+ status = acpi_dev_uid_to_integer(ACPI_COMPANION(dev), &uid);
+ if (status)
+ ssp->port_id = -1;
+ else
+ ssp->port_id = uid;
+
+ return 0;
+}
+
static struct pxa2xx_spi_controller *
pxa2xx_spi_init_pdata(struct platform_device *pdev)
{
struct pxa2xx_spi_controller *pdata;
struct device *dev = &pdev->dev;
struct device *parent = dev->parent;
- struct ssp_device *ssp;
- struct resource *res;
enum pxa_ssp_type type = SSP_UNDEFINED;
const void *match;
bool is_lpss_priv;
int status;
- u64 uid;
is_lpss_priv = platform_get_resource_byname(pdev, IORESOURCE_MEM, "lpss_priv");
@@ -1351,14 +1382,6 @@ pxa2xx_spi_init_pdata(struct platform_device *pdev)
if (!pdata)
return ERR_PTR(-ENOMEM);
- ssp = &pdata->ssp;
-
- ssp->mmio_base = devm_platform_get_and_ioremap_resource(pdev, 0, &res);
- if (IS_ERR(ssp->mmio_base))
- return ERR_CAST(ssp->mmio_base);
-
- ssp->phys_base = res->start;
-
/* Platforms with iDMA 64-bit */
if (is_lpss_priv) {
pdata->tx_param = parent;
@@ -1366,28 +1389,15 @@ pxa2xx_spi_init_pdata(struct platform_device *pdev)
pdata->dma_filter = pxa2xx_spi_idma_filter;
}
- ssp->clk = devm_clk_get(dev, NULL);
- if (IS_ERR(ssp->clk))
- return ERR_CAST(ssp->clk);
-
- ssp->irq = platform_get_irq(pdev, 0);
- if (ssp->irq < 0)
- return ERR_PTR(ssp->irq);
-
- ssp->type = type;
- ssp->dev = dev;
-
- status = acpi_dev_uid_to_integer(ACPI_COMPANION(dev), &uid);
- if (status)
- ssp->port_id = -1;
- else
- ssp->port_id = uid;
-
pdata->is_target = device_property_read_bool(dev, "spi-slave");
pdata->num_chipselect = 1;
pdata->enable_dma = true;
pdata->dma_burst_size = 1;
+ status = pxa2xx_spi_init_ssp(pdev, &pdata->ssp, type);
+ if (status)
+ return ERR_PTR(status);
+
return pdata;
}
--
2.43.0.rc1.1.gbec44491f096
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v1 07/10] spi: pxa2xx: Provide num-cs for Sharp PDAs via device properties
From: Andy Shevchenko @ 2024-03-26 18:07 UTC (permalink / raw)
To: Andy Shevchenko, Mark Brown, linux-spi, linux-kernel,
linux-arm-kernel
Cc: Daniel Mack, Haojian Zhuang, Robert Jarzmik, Russell King
In-Reply-To: <20240326181027.1418989-1-andriy.shevchenko@linux.intel.com>
Since driver can parse num-cs device property, replace platform data
with this new approach.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
arch/arm/mach-pxa/spitz.c | 25 ++++++++++++++++++-------
1 file changed, 18 insertions(+), 7 deletions(-)
diff --git a/arch/arm/mach-pxa/spitz.c b/arch/arm/mach-pxa/spitz.c
index 318402ad685e..3c5f5a3cb480 100644
--- a/arch/arm/mach-pxa/spitz.c
+++ b/arch/arm/mach-pxa/spitz.c
@@ -18,10 +18,10 @@
#include <linux/i2c.h>
#include <linux/platform_data/i2c-pxa.h>
#include <linux/platform_data/pca953x.h>
+#include <linux/property.h>
#include <linux/spi/spi.h>
#include <linux/spi/ads7846.h>
#include <linux/spi/corgi_lcd.h>
-#include <linux/spi/pxa2xx_spi.h>
#include <linux/mtd/sharpsl.h>
#include <linux/mtd/physmap.h>
#include <linux/input-event-codes.h>
@@ -569,10 +569,6 @@ static struct spi_board_info spitz_spi_devices[] = {
},
};
-static struct pxa2xx_spi_controller spitz_spi_info = {
- .num_chipselect = 3,
-};
-
static struct gpiod_lookup_table spitz_spi_gpio_table = {
.dev_id = "spi2",
.table = {
@@ -583,10 +579,20 @@ static struct gpiod_lookup_table spitz_spi_gpio_table = {
},
};
+static const struct property_entry spitz_spi_properties[] = {
+ PROPERTY_ENTRY_U32("num-cs", 3),
+ { }
+};
+
+static const struct software_node spitz_spi_node = {
+ .properties = spitz_spi_properties,
+};
+
static void __init spitz_spi_init(void)
{
struct platform_device *pd;
int id = 2;
+ int err;
if (machine_is_akita())
gpiod_add_lookup_table(&akita_lcdcon_gpio_table);
@@ -601,8 +607,13 @@ static void __init spitz_spi_init(void)
if (pd == NULL) {
pr_err("pxa2xx-spi: failed to allocate device id %d\n", id);
} else {
- pd->dev.platform_data = &spitz_spi_info;
- platform_device_add(pd);
+ err = device_add_software_node(&pd->dev, &spitz_spi_node);
+ if (err) {
+ platform_device_put(pd);
+ pr_err("pxa2xx-spi: failed to add software node\n");
+ } else {
+ platform_device_add(pd);
+ }
}
spi_register_board_info(ARRAY_AND_SIZE(spitz_spi_devices));
--
2.43.0.rc1.1.gbec44491f096
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v1 05/10] spi: pxa2xx: Skip SSP initialization if it's done elsewhere
From: Andy Shevchenko @ 2024-03-26 18:07 UTC (permalink / raw)
To: Andy Shevchenko, Mark Brown, linux-spi, linux-kernel,
linux-arm-kernel
Cc: Daniel Mack, Haojian Zhuang, Robert Jarzmik, Russell King
In-Reply-To: <20240326181027.1418989-1-andriy.shevchenko@linux.intel.com>
If SSP has been enumerated elsewhere, skip its initialization
in pxa2xx_spi_init_pdata().
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/spi/spi-pxa2xx.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/spi/spi-pxa2xx.c b/drivers/spi/spi-pxa2xx.c
index e7072727c25c..b01a18c89b6b 100644
--- a/drivers/spi/spi-pxa2xx.c
+++ b/drivers/spi/spi-pxa2xx.c
@@ -1355,6 +1355,7 @@ pxa2xx_spi_init_pdata(struct platform_device *pdev)
struct device *dev = &pdev->dev;
struct device *parent = dev->parent;
enum pxa_ssp_type type = SSP_UNDEFINED;
+ struct ssp_device *ssp = NULL;
const void *match;
bool is_lpss_priv;
int status;
@@ -1372,6 +1373,10 @@ pxa2xx_spi_init_pdata(struct platform_device *pdev)
return ERR_PTR(status);
type = (enum pxa_ssp_type)value;
+ } else {
+ ssp = pxa_ssp_request(pdev->id, pdev->name);
+ if (ssp)
+ type = ssp->type;
}
/* Validate the SSP type correctness */
@@ -1394,6 +1399,10 @@ pxa2xx_spi_init_pdata(struct platform_device *pdev)
pdata->enable_dma = true;
pdata->dma_burst_size = 1;
+ /* If SSP has been already enumerated, use it */
+ if (ssp)
+ return pdata;
+
status = pxa2xx_spi_init_ssp(pdev, &pdata->ssp, type);
if (status)
return ERR_PTR(status);
--
2.43.0.rc1.1.gbec44491f096
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v1 08/10] spi: pxa2xx: Move contents of linux/spi/pxa2xx_spi.h to a local one
From: Andy Shevchenko @ 2024-03-26 18:07 UTC (permalink / raw)
To: Andy Shevchenko, Mark Brown, linux-spi, linux-kernel,
linux-arm-kernel
Cc: Daniel Mack, Haojian Zhuang, Robert Jarzmik, Russell King,
Arnd Bergmann
In-Reply-To: <20240326181027.1418989-1-andriy.shevchenko@linux.intel.com>
There is no user of the linux/spi/pxa2xx_spi.h. Move its contents
to the drivers/spi/spi-pxa2xx.h.
Suggested-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/spi/spi-pxa2xx-dma.c | 1 -
drivers/spi/spi-pxa2xx-pci.c | 4 +--
drivers/spi/spi-pxa2xx.c | 1 -
drivers/spi/spi-pxa2xx.h | 36 ++++++++++++++++++++++++-
include/linux/spi/pxa2xx_spi.h | 48 ----------------------------------
5 files changed, 37 insertions(+), 53 deletions(-)
delete mode 100644 include/linux/spi/pxa2xx_spi.h
diff --git a/drivers/spi/spi-pxa2xx-dma.c b/drivers/spi/spi-pxa2xx-dma.c
index be563f0dd03a..26416ced6505 100644
--- a/drivers/spi/spi-pxa2xx-dma.c
+++ b/drivers/spi/spi-pxa2xx-dma.c
@@ -12,7 +12,6 @@
#include <linux/scatterlist.h>
#include <linux/sizes.h>
-#include <linux/spi/pxa2xx_spi.h>
#include <linux/spi/spi.h>
#include "spi-pxa2xx.h"
diff --git a/drivers/spi/spi-pxa2xx-pci.c b/drivers/spi/spi-pxa2xx-pci.c
index 861b21c63504..e11a613bc340 100644
--- a/drivers/spi/spi-pxa2xx-pci.c
+++ b/drivers/spi/spi-pxa2xx-pci.c
@@ -10,11 +10,11 @@
#include <linux/pci.h>
#include <linux/platform_device.h>
-#include <linux/spi/pxa2xx_spi.h>
-
#include <linux/dmaengine.h>
#include <linux/platform_data/dma-dw.h>
+#include "spi-pxa2xx.h"
+
#define PCI_DEVICE_ID_INTEL_QUARK_X1000 0x0935
#define PCI_DEVICE_ID_INTEL_BYT 0x0f0e
#define PCI_DEVICE_ID_INTEL_MRFLD 0x1194
diff --git a/drivers/spi/spi-pxa2xx.c b/drivers/spi/spi-pxa2xx.c
index f4435c39d096..e22d9d29c7e9 100644
--- a/drivers/spi/spi-pxa2xx.c
+++ b/drivers/spi/spi-pxa2xx.c
@@ -24,7 +24,6 @@
#include <linux/property.h>
#include <linux/slab.h>
-#include <linux/spi/pxa2xx_spi.h>
#include <linux/spi/spi.h>
#include "spi-pxa2xx.h"
diff --git a/drivers/spi/spi-pxa2xx.h b/drivers/spi/spi-pxa2xx.h
index 45cdbbc71c4b..08296729ea80 100644
--- a/drivers/spi/spi-pxa2xx.h
+++ b/drivers/spi/spi-pxa2xx.h
@@ -7,6 +7,7 @@
#ifndef SPI_PXA2XX_H
#define SPI_PXA2XX_H
+#include <linux/dmaengine.h>
#include <linux/interrupt.h>
#include <linux/io.h>
#include <linux/types.h>
@@ -15,7 +16,40 @@
#include <linux/pxa2xx_ssp.h>
struct gpio_desc;
-struct pxa2xx_spi_controller;
+
+/*
+ * The platform data for SSP controller devices
+ * (resides in device.platform_data).
+ */
+struct pxa2xx_spi_controller {
+ u8 num_chipselect;
+ u8 enable_dma;
+ u8 dma_burst_size;
+ bool is_target;
+
+ /* DMA engine specific config */
+ dma_filter_fn dma_filter;
+ void *tx_param;
+ void *rx_param;
+
+ /* For non-PXA arches */
+ struct ssp_device ssp;
+};
+
+/*
+ * The controller specific data for SPI target devices
+ * (resides in spi_board_info.controller_data),
+ * copied to spi_device.platform_data ... mostly for
+ * DMA tuning.
+ */
+struct pxa2xx_spi_chip {
+ u8 tx_threshold;
+ u8 tx_hi_threshold;
+ u8 rx_threshold;
+ u8 dma_burst_size;
+ u32 timeout;
+};
+
struct spi_controller;
struct spi_device;
struct spi_transfer;
diff --git a/include/linux/spi/pxa2xx_spi.h b/include/linux/spi/pxa2xx_spi.h
deleted file mode 100644
index e5a4a045fb67..000000000000
--- a/include/linux/spi/pxa2xx_spi.h
+++ /dev/null
@@ -1,48 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-or-later */
-/*
- * Copyright (C) 2005 Stephen Street / StreetFire Sound Labs
- */
-#ifndef __LINUX_SPI_PXA2XX_SPI_H
-#define __LINUX_SPI_PXA2XX_SPI_H
-
-#include <linux/dmaengine.h>
-#include <linux/types.h>
-
-#include <linux/pxa2xx_ssp.h>
-
-struct dma_chan;
-
-/*
- * The platform data for SSP controller devices
- * (resides in device.platform_data).
- */
-struct pxa2xx_spi_controller {
- u8 num_chipselect;
- u8 enable_dma;
- u8 dma_burst_size;
- bool is_target;
-
- /* DMA engine specific config */
- dma_filter_fn dma_filter;
- void *tx_param;
- void *rx_param;
-
- /* For non-PXA arches */
- struct ssp_device ssp;
-};
-
-/*
- * The controller specific data for SPI target devices
- * (resides in spi_board_info.controller_data),
- * copied to spi_device.platform_data ... mostly for
- * DMA tuning.
- */
-struct pxa2xx_spi_chip {
- u8 tx_threshold;
- u8 tx_hi_threshold;
- u8 rx_threshold;
- u8 dma_burst_size;
- u32 timeout;
-};
-
-#endif /* __LINUX_SPI_PXA2XX_SPI_H */
--
2.43.0.rc1.1.gbec44491f096
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v1 09/10] spi: pxa2xx: Remove outdated documentation
From: Andy Shevchenko @ 2024-03-26 18:07 UTC (permalink / raw)
To: Andy Shevchenko, Mark Brown, linux-spi, linux-kernel,
linux-arm-kernel
Cc: Daniel Mack, Haojian Zhuang, Robert Jarzmik, Russell King
In-Reply-To: <20240326181027.1418989-1-andriy.shevchenko@linux.intel.com>
The documentation is referring to the legacy enumeration of the SPI
host controllers and target devices. It has nothing to do with the
modern way, which is the only supported in kernel right now. Hence,
remove outdated documentation file.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
Documentation/spi/pxa2xx.rst | 208 -----------------------------------
1 file changed, 208 deletions(-)
delete mode 100644 Documentation/spi/pxa2xx.rst
diff --git a/Documentation/spi/pxa2xx.rst b/Documentation/spi/pxa2xx.rst
deleted file mode 100644
index 33d2ad95901e..000000000000
--- a/Documentation/spi/pxa2xx.rst
+++ /dev/null
@@ -1,208 +0,0 @@
-==============================
-PXA2xx SPI on SSP driver HOWTO
-==============================
-
-This a mini HOWTO on the pxa2xx_spi driver. The driver turns a PXA2xx
-synchronous serial port into an SPI host controller
-(see Documentation/spi/spi-summary.rst). The driver has the following features
-
-- Support for any PXA2xx and compatible SSP.
-- SSP PIO and SSP DMA data transfers.
-- External and Internal (SSPFRM) chip selects.
-- Per peripheral device (chip) configuration.
-- Full suspend, freeze, resume support.
-
-The driver is built around a &struct spi_message FIFO serviced by kernel
-thread. The kernel thread, spi_pump_messages(), drives message FIFO and
-is responsible for queuing SPI transactions and setting up and launching
-the DMA or interrupt driven transfers.
-
-Declaring PXA2xx host controllers
----------------------------------
-Typically, for a legacy platform, an SPI host controller is defined in the
-arch/.../mach-*/board-*.c as a "platform device". The host controller configuration
-is passed to the driver via a table found in include/linux/spi/pxa2xx_spi.h::
-
- struct pxa2xx_spi_controller {
- u8 num_chipselect;
- u8 enable_dma;
- ...
- };
-
-The "pxa2xx_spi_controller.num_chipselect" field is used to determine the number of
-peripheral devices (chips) attached to this SPI host controller.
-
-The "pxa2xx_spi_controller.enable_dma" field informs the driver that SSP DMA should
-be used. This caused the driver to acquire two DMA channels: Rx channel and
-Tx channel. The Rx channel has a higher DMA service priority than the Tx channel.
-See the "PXA2xx Developer Manual" section "DMA Controller".
-
-For the new platforms the description of the controller and peripheral devices
-comes from Device Tree or ACPI.
-
-NSSP HOST SAMPLE
-----------------
-Below is a sample configuration using the PXA255 NSSP for a legacy platform::
-
- static struct resource pxa_spi_nssp_resources[] = {
- [0] = {
- .start = __PREG(SSCR0_P(2)), /* Start address of NSSP */
- .end = __PREG(SSCR0_P(2)) + 0x2c, /* Range of registers */
- .flags = IORESOURCE_MEM,
- },
- [1] = {
- .start = IRQ_NSSP, /* NSSP IRQ */
- .end = IRQ_NSSP,
- .flags = IORESOURCE_IRQ,
- },
- };
-
- static struct pxa2xx_spi_controller pxa_nssp_controller_info = {
- .num_chipselect = 1, /* Matches the number of chips attached to NSSP */
- .enable_dma = 1, /* Enables NSSP DMA */
- };
-
- static struct platform_device pxa_spi_nssp = {
- .name = "pxa2xx-spi", /* MUST BE THIS VALUE, so device match driver */
- .id = 2, /* Bus number, MUST MATCH SSP number 1..n */
- .resource = pxa_spi_nssp_resources,
- .num_resources = ARRAY_SIZE(pxa_spi_nssp_resources),
- .dev = {
- .platform_data = &pxa_nssp_controller_info, /* Passed to driver */
- },
- };
-
- static struct platform_device *devices[] __initdata = {
- &pxa_spi_nssp,
- };
-
- static void __init board_init(void)
- {
- (void)platform_add_device(devices, ARRAY_SIZE(devices));
- }
-
-Declaring peripheral devices
-----------------------------
-Typically, for a legacy platform, each SPI peripheral device (chip) is defined in the
-arch/.../mach-*/board-*.c using the "spi_board_info" structure found in
-"linux/spi/spi.h". See "Documentation/spi/spi-summary.rst" for additional
-information.
-
-Each peripheral device (chip) attached to the PXA2xx must provide specific chip configuration
-information via the structure "pxa2xx_spi_chip" found in
-"include/linux/spi/pxa2xx_spi.h". The PXA2xx host controller driver will use
-the configuration whenever the driver communicates with the peripheral
-device. All fields are optional.
-
-::
-
- struct pxa2xx_spi_chip {
- u8 tx_threshold;
- u8 rx_threshold;
- u8 dma_burst_size;
- u32 timeout;
- };
-
-The "pxa2xx_spi_chip.tx_threshold" and "pxa2xx_spi_chip.rx_threshold" fields are
-used to configure the SSP hardware FIFO. These fields are critical to the
-performance of pxa2xx_spi driver and misconfiguration will result in rx
-FIFO overruns (especially in PIO mode transfers). Good default values are::
-
- .tx_threshold = 8,
- .rx_threshold = 8,
-
-The range is 1 to 16 where zero indicates "use default".
-
-The "pxa2xx_spi_chip.dma_burst_size" field is used to configure PXA2xx DMA
-engine and is related the "spi_device.bits_per_word" field. Read and understand
-the PXA2xx "Developer Manual" sections on the DMA controller and SSP Controllers
-to determine the correct value. An SSP configured for byte-wide transfers would
-use a value of 8. The driver will determine a reasonable default if
-dma_burst_size == 0.
-
-The "pxa2xx_spi_chip.timeout" fields is used to efficiently handle
-trailing bytes in the SSP receiver FIFO. The correct value for this field is
-dependent on the SPI bus speed ("spi_board_info.max_speed_hz") and the specific
-peripheral device. Please note that the PXA2xx SSP 1 does not support trailing byte
-timeouts and must busy-wait any trailing bytes.
-
-NOTE: the SPI driver cannot control the chip select if SSPFRM is used, so the
-chipselect is dropped after each spi_transfer. Most devices need chip select
-asserted around the complete message. Use SSPFRM as a GPIO (through a descriptor)
-to accommodate these chips.
-
-
-NSSP PERIPHERAL SAMPLE
-----------------------
-For a legacy platform or in some other cases, the pxa2xx_spi_chip structure
-is passed to the pxa2xx_spi driver in the "spi_board_info.controller_data"
-field. Below is a sample configuration using the PXA255 NSSP.
-
-::
-
- static struct pxa2xx_spi_chip cs8415a_chip_info = {
- .tx_threshold = 8, /* SSP hardware FIFO threshold */
- .rx_threshold = 8, /* SSP hardware FIFO threshold */
- .dma_burst_size = 8, /* Byte wide transfers used so 8 byte bursts */
- .timeout = 235, /* See Intel documentation */
- };
-
- static struct pxa2xx_spi_chip cs8405a_chip_info = {
- .tx_threshold = 8, /* SSP hardware FIFO threshold */
- .rx_threshold = 8, /* SSP hardware FIFO threshold */
- .dma_burst_size = 8, /* Byte wide transfers used so 8 byte bursts */
- .timeout = 235, /* See Intel documentation */
- };
-
- static struct spi_board_info streetracer_spi_board_info[] __initdata = {
- {
- .modalias = "cs8415a", /* Name of spi_driver for this device */
- .max_speed_hz = 3686400, /* Run SSP as fast a possible */
- .bus_num = 2, /* Framework bus number */
- .chip_select = 0, /* Framework chip select */
- .platform_data = NULL; /* No spi_driver specific config */
- .controller_data = &cs8415a_chip_info, /* Host controller config */
- .irq = STREETRACER_APCI_IRQ, /* Peripheral device interrupt */
- },
- {
- .modalias = "cs8405a", /* Name of spi_driver for this device */
- .max_speed_hz = 3686400, /* Run SSP as fast a possible */
- .bus_num = 2, /* Framework bus number */
- .chip_select = 1, /* Framework chip select */
- .controller_data = &cs8405a_chip_info, /* Host controller config */
- .irq = STREETRACER_APCI_IRQ, /* Peripheral device interrupt */
- },
- };
-
- static void __init streetracer_init(void)
- {
- spi_register_board_info(streetracer_spi_board_info,
- ARRAY_SIZE(streetracer_spi_board_info));
- }
-
-
-DMA and PIO I/O Support
------------------------
-The pxa2xx_spi driver supports both DMA and interrupt driven PIO message
-transfers. The driver defaults to PIO mode and DMA transfers must be enabled
-by setting the "enable_dma" flag in the "pxa2xx_spi_controller" structure.
-For the newer platforms, that are known to support DMA, the driver will enable
-it automatically and try it first with a possible fallback to PIO. The DMA
-mode supports both coherent and stream based DMA mappings.
-
-The following logic is used to determine the type of I/O to be used on
-a per "spi_transfer" basis::
-
- if spi_message.len > 65536 then
- print "rate limited" warning
- use PIO transfers
-
- if enable_dma and the size is in the range [DMA burst size..65536] then
- use streaming DMA mode
-
- otherwise
- use PIO transfer
-
-THANKS TO
----------
-David Brownell and others for mentoring the development of this driver.
--
2.43.0.rc1.1.gbec44491f096
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v1 10/10] spi: pxa2xx: Don't use "proxy" headers
From: Andy Shevchenko @ 2024-03-26 18:08 UTC (permalink / raw)
To: Andy Shevchenko, Mark Brown, linux-spi, linux-kernel,
linux-arm-kernel
Cc: Daniel Mack, Haojian Zhuang, Robert Jarzmik, Russell King
In-Reply-To: <20240326181027.1418989-1-andriy.shevchenko@linux.intel.com>
Update header inclusions to follow IWYU (Include What You Use)
principle.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/spi/spi-pxa2xx-dma.c | 10 ++++++++--
drivers/spi/spi-pxa2xx-pci.c | 6 ++++++
drivers/spi/spi-pxa2xx.c | 10 +++++++---
drivers/spi/spi-pxa2xx.h | 3 +--
4 files changed, 22 insertions(+), 7 deletions(-)
diff --git a/drivers/spi/spi-pxa2xx-dma.c b/drivers/spi/spi-pxa2xx-dma.c
index 26416ced6505..751cb0f77b62 100644
--- a/drivers/spi/spi-pxa2xx-dma.c
+++ b/drivers/spi/spi-pxa2xx-dma.c
@@ -6,16 +6,22 @@
* Author: Mika Westerberg <mika.westerberg@linux.intel.com>
*/
-#include <linux/device.h>
+#include <linux/atomic.h>
+#include <linux/dev_printk.h>
#include <linux/dma-mapping.h>
#include <linux/dmaengine.h>
+#include <linux/errno.h>
+#include <linux/irqreturn.h>
#include <linux/scatterlist.h>
-#include <linux/sizes.h>
+#include <linux/string.h>
+#include <linux/types.h>
#include <linux/spi/spi.h>
#include "spi-pxa2xx.h"
+struct device;
+
static void pxa2xx_spi_dma_transfer_complete(struct driver_data *drv_data,
bool error)
{
diff --git a/drivers/spi/spi-pxa2xx-pci.c b/drivers/spi/spi-pxa2xx-pci.c
index e11a613bc340..6d2efdb0e95f 100644
--- a/drivers/spi/spi-pxa2xx-pci.c
+++ b/drivers/spi/spi-pxa2xx-pci.c
@@ -6,9 +6,15 @@
* Copyright (C) 2016, 2021 Intel Corporation
*/
#include <linux/clk-provider.h>
+#include <linux/device.h>
+#include <linux/err.h>
#include <linux/module.h>
#include <linux/pci.h>
#include <linux/platform_device.h>
+#include <linux/property.h>
+#include <linux/sprintf.h>
+#include <linux/string.h>
+#include <linux/types.h>
#include <linux/dmaengine.h>
#include <linux/platform_data/dma-dw.h>
diff --git a/drivers/spi/spi-pxa2xx.c b/drivers/spi/spi-pxa2xx.c
index e22d9d29c7e9..86d0f1064a45 100644
--- a/drivers/spi/spi-pxa2xx.c
+++ b/drivers/spi/spi-pxa2xx.c
@@ -5,24 +5,28 @@
*/
#include <linux/acpi.h>
+#include <linux/atomic.h>
#include <linux/bitops.h>
+#include <linux/bug.h>
#include <linux/clk.h>
#include <linux/delay.h>
#include <linux/device.h>
#include <linux/dmaengine.h>
#include <linux/err.h>
-#include <linux/errno.h>
#include <linux/gpio/consumer.h>
#include <linux/init.h>
#include <linux/interrupt.h>
+#include <linux/io.h>
#include <linux/ioport.h>
-#include <linux/kernel.h>
-#include <linux/module.h>
+#include <linux/math64.h>
+#include <linux/minmax.h>
#include <linux/mod_devicetable.h>
+#include <linux/module.h>
#include <linux/platform_device.h>
#include <linux/pm_runtime.h>
#include <linux/property.h>
#include <linux/slab.h>
+#include <linux/types.h>
#include <linux/spi/spi.h>
diff --git a/drivers/spi/spi-pxa2xx.h b/drivers/spi/spi-pxa2xx.h
index 08296729ea80..f1097c96c50f 100644
--- a/drivers/spi/spi-pxa2xx.h
+++ b/drivers/spi/spi-pxa2xx.h
@@ -8,8 +8,7 @@
#define SPI_PXA2XX_H
#include <linux/dmaengine.h>
-#include <linux/interrupt.h>
-#include <linux/io.h>
+#include <linux/irqreturn.h>
#include <linux/types.h>
#include <linux/sizes.h>
--
2.43.0.rc1.1.gbec44491f096
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH v1 01/10] spi: pxa2xx: Drop ACPI_PTR() and of_match_ptr()
From: Mark Brown @ 2024-03-26 18:16 UTC (permalink / raw)
To: Andy Shevchenko
Cc: linux-spi, linux-kernel, linux-arm-kernel, Daniel Mack,
Haojian Zhuang, Robert Jarzmik, Russell King
In-Reply-To: <20240326181027.1418989-2-andriy.shevchenko@linux.intel.com>
[-- Attachment #1.1: Type: text/plain, Size: 371 bytes --]
On Tue, Mar 26, 2024 at 08:07:51PM +0200, Andy Shevchenko wrote:
> Drop rather useless use of ACPI_PTR() and of_match_ptr().
> It also removes the necessity to be dependent on ACPI and
> of.h inclusion.
I think the ACPI dependency there is as much about hiding the device on
irrelevant platforms as anything else, might be better replaced with an
x86 dependency though.
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2] spi: remove struct spi_message::is_dma_mapped
From: Andy Shevchenko @ 2024-03-26 17:06 UTC (permalink / raw)
To: David Lechner
Cc: Alexandre Belloni, Ryan Wanner, linux-kernel, Haojian Zhuang,
Claudiu Beznea, Mark Brown, linux-arm-kernel, linux-spi,
Robert Jarzmik, Daniel Mack
In-Reply-To: <20240325-spi-remove-is_dma_mapped-v2-1-d08d62b61f1c@baylibre.com>
On Mon, Mar 25, 2024 at 02:22:53PM -0500, David Lechner wrote:
> There are no more users of the deprecated is_dma_mapped in struct
> spi_message so it can be removed.
>
> References in documentation and comments are also removed.
>
> A few similar checks if xfer->tx_dma or xfer->rx_dma are not NULL are
> also removed since these are now guaranteed to be NULL because they
> were previously set only if is_dma_mapped was true.
Nice cleanup, thank you!
--
With Best Regards,
Andy Shevchenko
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v1 07/10] spi: pxa2xx: Provide num-cs for Sharp PDAs via device properties
From: Mark Brown @ 2024-03-26 18:21 UTC (permalink / raw)
To: Andy Shevchenko
Cc: linux-spi, linux-kernel, linux-arm-kernel, Daniel Mack,
Haojian Zhuang, Robert Jarzmik, Russell King
In-Reply-To: <20240326181027.1418989-8-andriy.shevchenko@linux.intel.com>
[-- Attachment #1.1: Type: text/plain, Size: 437 bytes --]
On Tue, Mar 26, 2024 at 08:07:57PM +0200, Andy Shevchenko wrote:
> Since driver can parse num-cs device property, replace platform data
> with this new approach.
But why?
> -static struct pxa2xx_spi_controller spitz_spi_info = {
> - .num_chipselect = 3,
> -};
> +static const struct property_entry spitz_spi_properties[] = {
> + PROPERTY_ENTRY_U32("num-cs", 3),
> + { }
> +};
This is just platform data with less validation AFAICT.
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 1/5] dt-bindings: iio: dac: ti,dac5571: Add DAC081C081 support
From: Jonathan Cameron @ 2024-03-26 18:21 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Laurent Pinchart, devicetree, imx, linux-arm-kernel,
Trevor Zaharichuk, Greg Lytle, Lars-Peter Clausen, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Sean Nyekjaer, linux-iio
In-Reply-To: <20240326171316.000018f5@Huawei.com>
On Tue, 26 Mar 2024 17:13:16 +0000
Jonathan Cameron <Jonathan.Cameron@Huawei.com> wrote:
> On Tue, 26 Mar 2024 18:50:43 +0200
> Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote:
>
> > On Tue, Mar 26, 2024 at 03:29:27PM +0000, Jonathan Cameron wrote:
> > > On Mon, 25 Mar 2024 22:56:41 +0200 Laurent Pinchart wrote:
> > > > On Mon, Mar 25, 2024 at 08:48:57PM +0000, Jonathan Cameron wrote:
> > > > > On Mon, 25 Mar 2024 22:32:41 +0200 Laurent Pinchart wrote:
> > > > >
> > > > > > The DAC081C081 is a TI DAC whose software interface is compatible with
> > > > > > the DAC5571. It is the 8-bit version of the DAC121C081, already
> > > > > > supported by the DAC5571 bindings. Extends the bindings to support this
> > > > > > chip.
> > > > > >
> > > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > > >
> > > > > Hi Laurent,
> > > > >
> > > > > Given it's a part number where no one is going to guess it is compatible
> > > > > with the DAC5571 and that we don't have a history of fallback compatibles
> > > > > I'm fine with this change, but just wanted to ask is a fallback compatible
> > > > > useful to you to run with older kernels?
> > > > >
> > > > > I should have noticed when Peter added the dac121c081. If we add a fallback
> > > > > should do that one as well.
> > > >
> > > > I've indeed noticed that there should have been a fallback for
> > > > dac121c081, but didn't stop to ponder why that wasn't the case, and just
> > > > went along with the flow :-) I agree a fallback could be useful, which
> > > > would then allow dropping patch 2/5 from this series (*). I can do so if
> > > > you prefer.
> > > >
> > > > * This is not entirely true. While the DAC1081C081 is largely compatible
> > > > with the DAC5573, they have different values for one of the power-down
> > > > resistors (2.5kΩ instead of 1kΩ if I recall correctly). To be completely
> > > > accurate, the driver should report that. We could still use the fallback
> > > > compatible, reporting the wrong power-down resistor value.
> > >
> > > Hmm - Would anyone really care about that value being wrong?
> >
> > I don't have enough expertise with IIO to be sure, but my guess is that
> > nobody would.
> >
> > > I think perhaps that's just about significant enough that maybe a fallback
> > > compatible doesn't make sense here.
> >
> > Then let's keep it simple and just merge this patch as-is ? :-)
> Makes sense. I'm on wrong computer at the moment but will pick it up at
> weekend if not before.
Sooner it is. Applied 1 and 2 to the togreg branch of iio.git and pushed
out as testing on the extremely unlikely chance that 0-day finds a build
issue...
I grabbed them today because I have a memory like a gold fish and the thread
is deep enough that I might skip over later on basis 'it must have outstanding
questions' :)
Jonathan
>
> Jonathan
>
>
> >
> > > > > > ---
> > > > > > Documentation/devicetree/bindings/iio/dac/ti,dac5571.yaml | 1 +
> > > > > > 1 file changed, 1 insertion(+)
> > > > > >
> > > > > > diff --git a/Documentation/devicetree/bindings/iio/dac/ti,dac5571.yaml b/Documentation/devicetree/bindings/iio/dac/ti,dac5571.yaml
> > > > > > index 79da0323c327..e59db861e2eb 100644
> > > > > > --- a/Documentation/devicetree/bindings/iio/dac/ti,dac5571.yaml
> > > > > > +++ b/Documentation/devicetree/bindings/iio/dac/ti,dac5571.yaml
> > > > > > @@ -21,6 +21,7 @@ properties:
> > > > > > - ti,dac5573
> > > > > > - ti,dac6573
> > > > > > - ti,dac7573
> > > > > > + - ti,dac081c081
> > > > > > - ti,dac121c081
> > > > > >
> > > > > > reg:
> >
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v1 01/10] spi: pxa2xx: Drop ACPI_PTR() and of_match_ptr()
From: Andy Shevchenko @ 2024-03-26 18:22 UTC (permalink / raw)
To: Mark Brown
Cc: linux-spi, linux-kernel, linux-arm-kernel, Daniel Mack,
Haojian Zhuang, Robert Jarzmik, Russell King
In-Reply-To: <14f08a50-edef-4b36-891e-2b4b2283f40c@sirena.org.uk>
On Tue, Mar 26, 2024 at 06:16:28PM +0000, Mark Brown wrote:
> On Tue, Mar 26, 2024 at 08:07:51PM +0200, Andy Shevchenko wrote:
> > Drop rather useless use of ACPI_PTR() and of_match_ptr().
> > It also removes the necessity to be dependent on ACPI and
> > of.h inclusion.
>
> I think the ACPI dependency there is as much about hiding the device on
> irrelevant platforms as anything else, might be better replaced with an
> x86 dependency though.
The whole idea behind ACPI_PTR() (which seems following the of_match_ptr()
implementation) looks premature. Now we have a lot of patches from DT people to
remove of_match_ptr(), i.o.w. make the ID visible even on non-OF platforms.
Having the list of supported IDs is not bad thing anyway as it might help
to google for a device elsewhere, for example.
--
With Best Regards,
Andy Shevchenko
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v6 1/2] dt-bindings: arm: aspeed: add ASUS X4TF board
From: Conor Dooley @ 2024-03-26 18:24 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Kelly Hung, robh+dt, krzysztof.kozlowski+dt, conor+dt, joel,
andrew, devicetree, linux-arm-kernel, linux-aspeed, linux-kernel,
openbmc, kelly_hung, Allenyy_Hsu, Rob Herring, Zev Weiss,
Conor Dooley
In-Reply-To: <32cd6f33-b4e9-4b7a-bcea-b1f2e421d67e@linaro.org>
[-- Attachment #1.1: Type: text/plain, Size: 946 bytes --]
On Tue, Mar 26, 2024 at 12:36:50PM +0100, Krzysztof Kozlowski wrote:
> On 26/03/2024 11:35, Kelly Hung wrote:
> > Document the new compatibles used on ASUS X4TF.
> >
> > Signed-off-by: Kelly Hung <Kelly_Hung@asus.com>
> > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> > Reviewed-by: Rob Herring <robh@kernel.org>
>
> Where did it happen? Where did you receive this tag? Please provide link
> to lore.
Robh's bot sent two reports, but I do not see a tag:
https://lore.kernel.org/all/?q=c%3AKelly_Hung%40asus.com+f%3Arobh
> > Reviewed-by: Zev Weiss <zweiss@equinix.com>
>
> Where did it happen? Where did you receive this tag? Please provide link
> to lore.
Zev seems to have left a comment on this, but not provided a tag:
https://lore.kernel.org/all/20240223220115.GB9299@packtop/
Kelly, it is important that you do not add tags unless someone gives
them to you explicitly.
Thanks,
Conor.
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v1 01/10] spi: pxa2xx: Drop ACPI_PTR() and of_match_ptr()
From: Mark Brown @ 2024-03-26 18:25 UTC (permalink / raw)
To: Andy Shevchenko
Cc: linux-spi, linux-kernel, linux-arm-kernel, Daniel Mack,
Haojian Zhuang, Robert Jarzmik, Russell King
In-Reply-To: <ZgMSg5Tr97mWgPW4@smile.fi.intel.com>
[-- Attachment #1.1: Type: text/plain, Size: 970 bytes --]
On Tue, Mar 26, 2024 at 08:22:59PM +0200, Andy Shevchenko wrote:
> On Tue, Mar 26, 2024 at 06:16:28PM +0000, Mark Brown wrote:
> > On Tue, Mar 26, 2024 at 08:07:51PM +0200, Andy Shevchenko wrote:
> > > Drop rather useless use of ACPI_PTR() and of_match_ptr().
> > > It also removes the necessity to be dependent on ACPI and
> > > of.h inclusion.
> > I think the ACPI dependency there is as much about hiding the device on
> > irrelevant platforms as anything else, might be better replaced with an
> > x86 dependency though.
> The whole idea behind ACPI_PTR() (which seems following the of_match_ptr()
> implementation) looks premature. Now we have a lot of patches from DT people to
> remove of_match_ptr(), i.o.w. make the ID visible even on non-OF platforms.
> Having the list of supported IDs is not bad thing anyway as it might help
> to google for a device elsewhere, for example.
That's nice but I'm not sure what that has to do with the dependency on
ACPI?
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v5 01/27] iommu/arm-smmu-v3: Do not allow a SVA domain to be set on the wrong PASID
From: Jason Gunthorpe @ 2024-03-26 18:30 UTC (permalink / raw)
To: Mostafa Saleh
Cc: iommu, Joerg Roedel, linux-arm-kernel, Robin Murphy, Will Deacon,
Eric Auger, Jean-Philippe Brucker, Moritz Fischer, Michael Shavit,
Nicolin Chen, patches, Shameerali Kolothum Thodi
In-Reply-To: <Zf3EhA5ZBNwF2XOr@google.com>
On Fri, Mar 22, 2024 at 05:48:52PM +0000, Mostafa Saleh wrote:
> Hi Jason,
>
> On Mon, Mar 04, 2024 at 07:43:49PM -0400, Jason Gunthorpe wrote:
> > The SVA code is wired to assume that the SVA is programmed onto the
> > mm->pasid. The current core code always does this, so it is fine.
> >
> > Add a check for clarity.
> >
> > Tested-by: Nicolin Chen <nicolinc@nvidia.com>
> > Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> > ---
> > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
> > index 2610e82c0ecd0d..347c2fdd865c1a 100644
> > --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
> > +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
> > @@ -581,6 +581,9 @@ static int arm_smmu_sva_set_dev_pasid(struct iommu_domain *domain,
> > int ret = 0;
> > struct mm_struct *mm = domain->mm;
> >
> > + if (mm_get_enqcmd_pasid(mm) != id)
> > + return -EINVAL;
> > +
> I am not sure if that is needed, the only caller in the tree is the IOMMU code
> and it does the right thing, as that check is removed later anyway, I don’t
> think this patch adds much.
It really should be backported, when we get drivers that do other
things it creates a hazard. I've added a fixes line.
Jason
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: arm: bcm: raspberrypi,bcm2835-firmware: Add missing properties
From: Dave Stevenson @ 2024-03-26 18:41 UTC (permalink / raw)
To: Stefan Wahren
Cc: Krzysztof Kozlowski, Laurent Pinchart, devicetree,
linux-rpi-kernel, linux-arm-kernel, linux-gpio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Florian Fainelli,
Broadcom internal kernel review list, Ray Jui, Scott Branden,
Linus Walleij, Bartosz Golaszewski
In-Reply-To: <45242028-edf7-49fc-80bf-be9eb242b4cd@gmx.net>
On Tue, 26 Mar 2024 at 11:47, Stefan Wahren <wahrenst@gmx.net> wrote:
>
> [add Dave since he's working on DMA for Raspberry Pi 4 and maybe have a
> opinion about this]
No real opinion from me, but I am far from a DT expert, and AFAIK this
bit isn't impacted by the stuff I'm looking at.
Laurent is correct that it's missing from the binding doc when it
looks to be needed. Adding it would therefore be the correct thing to
do.
Dave
> [drop Emma Anholt old address since she is not involved anymore]
>
> Am 26.03.24 um 08:06 schrieb Krzysztof Kozlowski:
> > On 26/03/2024 01:49, Laurent Pinchart wrote:
> >> The raspberrypi,bcm2835-firmware devices requires a dma-ranges property,
> >> and, as a result, also needs to specify #address-cells and #size-cells.
> >> Those properties have been added to thebcm2835-rpi.dtsi in commits
> >> be08d278eb09 ("ARM: dts: bcm283x: Add cells encoding format to firmware
> >> bus") and 55c7c0621078 ("ARM: dts: bcm283x: Fix vc4's firmware bus DMA
> >> limitations"), but the DT bindings haven't been updated, resulting in
> >> validation errors:
> >>
> >> arch/arm64/boot/dts/broadcom/bcm2711-rpi-4-b.dtb: firmware: '#address-cells', '#size-cells', 'dma-ranges', 'gpio' do not match any of the regexes: 'pinctrl-[0-9]+'
> >> from schema $id: http://devicetree.org/schemas/arm/bcm/raspberrypi,bcm2835-firmware.yaml#
> >>
> >> Fix this by adding the properties to the bindings.
> >>
> >> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > Children do not perform any IO on their own, because everything is
> > handled by parent. It is really odd to see dma-ranges without ranges.
> > Referenced commits might be also wrong.
> >
> > Best regards,
> > Krzysztof
> >
> >
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v1 01/10] spi: pxa2xx: Drop ACPI_PTR() and of_match_ptr()
From: Andy Shevchenko @ 2024-03-26 18:44 UTC (permalink / raw)
To: Mark Brown
Cc: linux-spi, linux-kernel, linux-arm-kernel, Daniel Mack,
Haojian Zhuang, Robert Jarzmik, Russell King
In-Reply-To: <424de3ed-f0ea-4fc1-80f5-3ab1d23cf1e1@sirena.org.uk>
On Tue, Mar 26, 2024 at 06:25:16PM +0000, Mark Brown wrote:
> On Tue, Mar 26, 2024 at 08:22:59PM +0200, Andy Shevchenko wrote:
> > On Tue, Mar 26, 2024 at 06:16:28PM +0000, Mark Brown wrote:
> > > On Tue, Mar 26, 2024 at 08:07:51PM +0200, Andy Shevchenko wrote:
>
> > > > Drop rather useless use of ACPI_PTR() and of_match_ptr().
> > > > It also removes the necessity to be dependent on ACPI and
> > > > of.h inclusion.
>
> > > I think the ACPI dependency there is as much about hiding the device on
> > > irrelevant platforms as anything else, might be better replaced with an
> > > x86 dependency though.
>
> > The whole idea behind ACPI_PTR() (which seems following the of_match_ptr()
> > implementation) looks premature. Now we have a lot of patches from DT people to
> > remove of_match_ptr(), i.o.w. make the ID visible even on non-OF platforms.
>
> > Having the list of supported IDs is not bad thing anyway as it might help
> > to google for a device elsewhere, for example.
>
> That's nice but I'm not sure what that has to do with the dependency on
> ACPI?
ACPI_PTR() makes ID no-op only if ACPI is defined. That also satisfies
the ugly ifdeffery that is removed by this patch as well. If there is
no dependency we will have compiler warning for defined but not used
variable.
--
With Best Regards,
Andy Shevchenko
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 04/25] clk: meson: a1: add the audio clock controller driver
From: Jan Dakinevich @ 2024-03-26 18:44 UTC (permalink / raw)
To: Jerome Brunet
Cc: Neil Armstrong, Michael Turquette, Stephen Boyd, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Philipp Zabel, Kevin Hilman,
Martin Blumenstingl, Liam Girdwood, Mark Brown, Linus Walleij,
Jaroslav Kysela, Takashi Iwai, linux-amlogic, linux-clk,
devicetree, linux-kernel, linux-arm-kernel, alsa-devel,
linux-sound, linux-gpio, kernel
In-Reply-To: <1jy1a580g0.fsf@starbuckisacylon.baylibre.com>
On 3/26/24 18:26, Jerome Brunet wrote:
>
> On Sat 23 Mar 2024 at 21:02, Jan Dakinevich <jan.dakinevich@salutedevices.com> wrote:
>
>> Jerome, I have reworked my driver reusing axg-audio code as most as I
>> could and now I have one more question. Lets see on this definition from
>> axg-audio:
>>
>> #define AUD_MST_MUX(_name, _reg, _flag) \
>> AUD_MUX(_name##_sel, _reg, 0x7, 24, _flag, \
>> mst_mux_parent_data, 0)
>>
>> #define AUD_MST_MCLK_MUX(_name, _reg) \
>> AUD_MST_MUX(_name, _reg, CLK_MUX_ROUND_CLOSEST)
>>
>> CLK_SET_RATE_PARENT is not set here. But why? It means, that topmost pll
>> clock will not be reconfigured at runtime to satisfy the rate that was
>> requested from axg-tdm.
>>
>
> Yes, that is by design. It is another area where mainline audio differs
> greatly from AML vendor code. The PLLs are expected be to fixed and the
> audio master clock will reparent to the most adequate PLL source
> depending on the use case.
>
> This is how we manage to satisfy all audio interfaces with a very
> limited number of PLLs
>
> On AXG/G12 there is at most 6 concurrent interfaces (3 FRDDR/TODDR) - 8
> on sm1 - and we can satisfy on that with 3 PLLs. That would not be
> possible if interfaces were having their way with the PLLs, reseting it
> everytime a stream is started.
> > The PLL rate should be carefully chosen so it can be derived easily. On
> AXG/G12/SM1 that is:
> * one PLL per rate family, to maximize clock precision
> * x24 x32: to handle different sample sizes
> * x2 until we reach the PLL limits to allow higher rates such as 384kHz
> or even higher
>
Thank you. Now it has become much clearer.
> If you have less PLLs on A1, you'll have to make compromises, like a less
> precise clock to support multiple family with one PLL.
> This is why the PLLs are set for each platform in DT because that choice
> may depend on the platform use case.
>
Unfortunately, on A1 we have only one PLL.
Yes, for us it would be better to have hifi_pll with predefined rate.
For instance it will allow to avoid that ugly workaround in PDM (sysrate
property, etc).
But what whould be preferred for upstream? I can imagine a scenario
where samples with different rate should be played, PDM attached to
fclk_divN and there are no conflicts with TDM. In this case
reconfiguration of hifi_pll on demand could better satisfy somebody's
requirements.
>>
>> On 3/19/24 11:30, Jerome Brunet wrote:
>>>
>>> On Tue 19 Mar 2024 at 04:47, Jan Dakinevich <jan.dakinevich@salutedevices.com> wrote:
>>>
>>>> Let's start from the end:
>>>>
>>>>> No - Looks to me you just have two clock controllers you are trying
>>>> force into one.
>>>>
>>>>> Again, this shows 2 devices. The one related to your 'map0' should
>>>> request AUD2_CLKID_AUDIOTOP as input and enable it right away.
>>>>
>>>> Most of fishy workarounds that you commented is caused the fact the mmio
>>>> of this clock controller is divided into two parts. Compare it with
>>>> axg-audio driver, things that was part of contigous memory region (like
>>>> pdm) here are moved to second region. Is this enough to make a guess
>>>> that these are two devices?
>>>
>>> I see obsolutely no reason to think it is a single device nor to add all the quirks
>>> you have the way you did. So yes, in that case, 2 zones, 2 devices.
>>>
>>>>
>>>> Concerning AUD2_CLKID_AUDIOTOP clock, as it turned out, it must be
>>>> enabled before enabling of clocks from second region too. That is
>>>> AUD2_CLKID_AUDIOTOP clock feeds both parts of this clock controller.
>>>>
>>>
>>> Yes. I understood the first time around and already commented on that.
>>>
>>>>
>>>> On 3/15/24 12:20, Jerome Brunet wrote:
>>>>>
>>>>> On Fri 15 Mar 2024 at 02:21, Jan Dakinevich <jan.dakinevich@salutedevices.com> wrote:
>>>>>
>>>>>> This controller provides clocks and reset functionality for audio
>>>>>> peripherals on Amlogic A1 SoC family.
>>>>>>
>>>>>> The driver is almost identical to 'axg-audio', however it would be better
>>>>>> to keep it separate due to following reasons:
>>>>>>
>>>>>> - significant amount of bits has another definition. I will bring there
>>>>>> a mess of new defines with A1_ suffixes.
>>>>>>
>>>>>> - registers of this controller are located in two separate regions. It
>>>>>> will give a lot of complications for 'axg-audio' to support this.
>>>>>>
>>>>>> Signed-off-by: Jan Dakinevich <jan.dakinevich@salutedevices.com>
>>>>>> ---
>>>>>> drivers/clk/meson/Kconfig | 13 +
>>>>>> drivers/clk/meson/Makefile | 1 +
>>>>>> drivers/clk/meson/a1-audio.c | 556 +++++++++++++++++++++++++++++++++++
>>>>>> drivers/clk/meson/a1-audio.h | 58 ++++
>>>>>> 4 files changed, 628 insertions(+)
>>>>>> create mode 100644 drivers/clk/meson/a1-audio.c
>>>>>> create mode 100644 drivers/clk/meson/a1-audio.h
>>>>>>
>>>>>> diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
>>>>>> index d6a2fa5f7e88..80c4a18c83d2 100644
>>>>>> --- a/drivers/clk/meson/Kconfig
>>>>>> +++ b/drivers/clk/meson/Kconfig
>>>>>> @@ -133,6 +133,19 @@ config COMMON_CLK_A1_PERIPHERALS
>>>>>> device, A1 SoC Family. Say Y if you want A1 Peripherals clock
>>>>>> controller to work.
>>>>>>
>>>>>> +config COMMON_CLK_A1_AUDIO
>>>>>> + tristate "Amlogic A1 SoC Audio clock controller support"
>>>>>> + depends on ARM64
>>>>>> + select COMMON_CLK_MESON_REGMAP
>>>>>> + select COMMON_CLK_MESON_CLKC_UTILS
>>>>>> + select COMMON_CLK_MESON_PHASE
>>>>>> + select COMMON_CLK_MESON_SCLK_DIV
>>>>>> + select COMMON_CLK_MESON_AUDIO_RSTC
>>>>>> + help
>>>>>> + Support for the Audio clock controller on Amlogic A113L based
>>>>>> + device, A1 SoC Family. Say Y if you want A1 Audio clock controller
>>>>>> + to work.
>>>>>> +
>>>>>> config COMMON_CLK_G12A
>>>>>> tristate "G12 and SM1 SoC clock controllers support"
>>>>>> depends on ARM64
>>>>>> diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
>>>>>> index 88d94921a4dc..4968fc7ad555 100644
>>>>>> --- a/drivers/clk/meson/Makefile
>>>>>> +++ b/drivers/clk/meson/Makefile
>>>>>> @@ -20,6 +20,7 @@ obj-$(CONFIG_COMMON_CLK_AXG) += axg.o axg-aoclk.o
>>>>>> obj-$(CONFIG_COMMON_CLK_AXG_AUDIO) += axg-audio.o
>>>>>> obj-$(CONFIG_COMMON_CLK_A1_PLL) += a1-pll.o
>>>>>> obj-$(CONFIG_COMMON_CLK_A1_PERIPHERALS) += a1-peripherals.o
>>>>>> +obj-$(CONFIG_COMMON_CLK_A1_AUDIO) += a1-audio.o
>>>>>> obj-$(CONFIG_COMMON_CLK_GXBB) += gxbb.o gxbb-aoclk.o
>>>>>> obj-$(CONFIG_COMMON_CLK_G12A) += g12a.o g12a-aoclk.o
>>>>>> obj-$(CONFIG_COMMON_CLK_MESON8B) += meson8b.o meson8-ddr.o
>>>>>> diff --git a/drivers/clk/meson/a1-audio.c b/drivers/clk/meson/a1-audio.c
>>>>>> new file mode 100644
>>>>>> index 000000000000..6039116c93ba
>>>>>> --- /dev/null
>>>>>> +++ b/drivers/clk/meson/a1-audio.c
>>>>>> @@ -0,0 +1,556 @@
>>>>>> +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
>>>>>> +/*
>>>>>> + * Copyright (c) 2024, SaluteDevices. All Rights Reserved.
>>>>>> + *
>>>>>> + * Author: Jan Dakinevich <jan.dakinevich@salutedevices.com>
>>>>>> + */
>>>>>> +
>>>>>> +#include <linux/clk.h>
>>>>>> +#include <linux/clk-provider.h>
>>>>>> +#include <linux/init.h>
>>>>>> +#include <linux/of_device.h>
>>>>>> +#include <linux/module.h>
>>>>>> +#include <linux/platform_device.h>
>>>>>> +#include <linux/regmap.h>
>>>>>> +#include <linux/reset.h>
>>>>>> +#include <linux/reset-controller.h>
>>>>>> +#include <linux/slab.h>
>>>>>> +
>>>>>> +#include "meson-clkc-utils.h"
>>>>>> +#include "meson-audio-rstc.h"
>>>>>> +#include "clk-regmap.h"
>>>>>> +#include "clk-phase.h"
>>>>>> +#include "sclk-div.h"
>>>>>> +#include "a1-audio.h"
>>>>>> +
>>>>>> +#define AUDIO_PDATA(_name) \
>>>>>> + ((const struct clk_parent_data[]) { { .hw = &(_name).hw } })
>>>>>
>>>>> Not a fan - yet another level of macro.
>>>>>
>>>>>> +
>>>>>> +#define AUDIO_MUX(_name, _reg, _mask, _shift, _pdata) \
>>>>>> +static struct clk_regmap _name = { \
>>>>>> + .map = AUDIO_REG_MAP(_reg), \
>>>>>> + .data = &(struct clk_regmap_mux_data){ \
>>>>>> + .offset = AUDIO_REG_OFFSET(_reg), \
>>>>>> + .mask = (_mask), \
>>>>>> + .shift = (_shift), \
>>>>>> + }, \
>>>>>> + .hw.init = &(struct clk_init_data) { \
>>>>>> + .name = #_name, \
>>>>>> + .ops = &clk_regmap_mux_ops, \
>>>>>> + .parent_data = (_pdata), \
>>>>>> + .num_parents = ARRAY_SIZE(_pdata), \
>>>>>> + .flags = CLK_SET_RATE_PARENT, \
>>>>>> + }, \
>>>>>> +}
>>>>>> +
>>>>>> +#define AUDIO_DIV(_name, _reg, _shift, _width, _pdata) \
>>>>>> +static struct clk_regmap _name = { \
>>>>>> + .map = AUDIO_REG_MAP(_reg), \
>>>>>> + .data = &(struct clk_regmap_div_data){ \
>>>>>> + .offset = AUDIO_REG_OFFSET(_reg), \
>>>>>> + .shift = (_shift), \
>>>>>> + .width = (_width), \
>>>>>> + }, \
>>>>>> + .hw.init = &(struct clk_init_data) { \
>>>>>> + .name = #_name, \
>>>>>> + .ops = &clk_regmap_divider_ops, \
>>>>>> + .parent_data = (_pdata), \
>>>>>> + .num_parents = 1, \
>>>>>> + .flags = CLK_SET_RATE_PARENT, \
>>>>>> + }, \
>>>>>> +}
>>>>>> +
>>>>>> +#define AUDIO_GATE(_name, _reg, _bit, _pdata) \
>>>>>> +static struct clk_regmap _name = { \
>>>>>> + .map = AUDIO_REG_MAP(_reg), \
>>>>>> + .data = &(struct clk_regmap_gate_data){ \
>>>>>> + .offset = AUDIO_REG_OFFSET(_reg), \
>>>>>> + .bit_idx = (_bit), \
>>>>>> + }, \
>>>>>> + .hw.init = &(struct clk_init_data) { \
>>>>>> + .name = #_name, \
>>>>>> + .ops = &clk_regmap_gate_ops, \
>>>>>> + .parent_data = (_pdata), \
>>>>>> + .num_parents = 1, \
>>>>>> + .flags = CLK_SET_RATE_PARENT, \
>>>>>> + }, \
>>>>>> +}
>>>>>> +
>>>>>> +#define AUDIO_SCLK_DIV(_name, _reg, _div_shift, _div_width, \
>>>>>> + _hi_shift, _hi_width, _pdata, _set_rate_parent) \
>>>>>> +static struct clk_regmap _name = { \
>>>>>> + .map = AUDIO_REG_MAP(_reg), \
>>>>>> + .data = &(struct meson_sclk_div_data) { \
>>>>>> + .div = { \
>>>>>> + .reg_off = AUDIO_REG_OFFSET(_reg), \
>>>>>> + .shift = (_div_shift), \
>>>>>> + .width = (_div_width), \
>>>>>> + }, \
>>>>>> + .hi = { \
>>>>>> + .reg_off = AUDIO_REG_OFFSET(_reg), \
>>>>>> + .shift = (_hi_shift), \
>>>>>> + .width = (_hi_width), \
>>>>>> + }, \
>>>>>> + }, \
>>>>>> + .hw.init = &(struct clk_init_data) { \
>>>>>> + .name = #_name, \
>>>>>> + .ops = &meson_sclk_div_ops, \
>>>>>> + .parent_data = (_pdata), \
>>>>>> + .num_parents = 1, \
>>>>>> + .flags = (_set_rate_parent) ? CLK_SET_RATE_PARENT : 0, \
>>>>>
>>>>> Does not help readeability. Just pass the flag as axg-audio does.
>>>>>
>>>>>> + }, \
>>>>>> +}
>>>>>> +
>>>>>> +#define AUDIO_TRIPHASE(_name, _reg, _width, _shift0, _shift1, _shift2, \
>>>>>> + _pdata) \
>>>>>> +static struct clk_regmap _name = { \
>>>>>> + .map = AUDIO_REG_MAP(_reg), \
>>>>>> + .data = &(struct meson_clk_triphase_data) { \
>>>>>> + .ph0 = { \
>>>>>> + .reg_off = AUDIO_REG_OFFSET(_reg), \
>>>>>> + .shift = (_shift0), \
>>>>>> + .width = (_width), \
>>>>>> + }, \
>>>>>> + .ph1 = { \
>>>>>> + .reg_off = AUDIO_REG_OFFSET(_reg), \
>>>>>> + .shift = (_shift1), \
>>>>>> + .width = (_width), \
>>>>>> + }, \
>>>>>> + .ph2 = { \
>>>>>> + .reg_off = AUDIO_REG_OFFSET(_reg), \
>>>>>> + .shift = (_shift2), \
>>>>>> + .width = (_width), \
>>>>>> + }, \
>>>>>> + }, \
>>>>>> + .hw.init = &(struct clk_init_data) { \
>>>>>> + .name = #_name, \
>>>>>> + .ops = &meson_clk_triphase_ops, \
>>>>>> + .parent_data = (_pdata), \
>>>>>> + .num_parents = 1, \
>>>>>> + .flags = CLK_SET_RATE_PARENT | CLK_DUTY_CYCLE_PARENT, \
>>>>>> + }, \
>>>>>> +}
>>>>>> +
>>>>>> +#define AUDIO_SCLK_WS(_name, _reg, _width, _shift_ph, _shift_ws, \
>>>>>> + _pdata) \
>>>>>> +static struct clk_regmap _name = { \
>>>>>> + .map = AUDIO_REG_MAP(_reg), \
>>>>>> + .data = &(struct meson_sclk_ws_inv_data) { \
>>>>>> + .ph = { \
>>>>>> + .reg_off = AUDIO_REG_OFFSET(_reg), \
>>>>>> + .shift = (_shift_ph), \
>>>>>> + .width = (_width), \
>>>>>> + }, \
>>>>>> + .ws = { \
>>>>>> + .reg_off = AUDIO_REG_OFFSET(_reg), \
>>>>>> + .shift = (_shift_ws), \
>>>>>> + .width = (_width), \
>>>>>> + }, \
>>>>>> + }, \
>>>>>> + .hw.init = &(struct clk_init_data) { \
>>>>>> + .name = #_name, \
>>>>>> + .ops = &meson_sclk_ws_inv_ops, \
>>>>>> + .parent_data = (_pdata), \
>>>>>> + .num_parents = 1, \
>>>>>> + .flags = CLK_SET_RATE_PARENT | CLK_DUTY_CYCLE_PARENT, \
>>>>>> + }, \
>>>>>> +}
>>>>>
>>>>> All the above does essentially the same things as the macro of
>>>>> axg-audio, to some minor differences. Yet it is another set to maintain.
>>>>>
>>>>
>>>> Except one thing... Here I keep memory identifier to which this clock
>>>> belongs:
>>>>
>>>> .map = AUDIO_REG_MAP(_reg),
>>>>
>>>> It is workaround, but ->map the only common field in clk_regmap that
>>>> could be used for this purpose.
>>>>
>>>>
>>>>> I'd much prefer if you put the axg-audio macro in a header a re-used
>>>>> those. There would a single set to maintain. You may then specialize the
>>>>> included in the driver C file, to avoid redundant parameters
>>>>>
>>>>> Rework axg-audio to use clk_parent_data if you must, but not in the same
>>>>> series please.
>>>>>
>>>>>> +
>>>>>> +static const struct clk_parent_data a1_pclk_pdata[] = {
>>>>>> + { .fw_name = "pclk", },
>>>>>> +};
>>>>>> +
>>>>>> +AUDIO_GATE(audio_ddr_arb, AUDIO_CLK_GATE_EN0, 0, a1_pclk_pdata);
>>>>>> +AUDIO_GATE(audio_tdmin_a, AUDIO_CLK_GATE_EN0, 1, a1_pclk_pdata);
>>>>>> +AUDIO_GATE(audio_tdmin_b, AUDIO_CLK_GATE_EN0, 2, a1_pclk_pdata);
>>>>>> +AUDIO_GATE(audio_tdmin_lb, AUDIO_CLK_GATE_EN0, 3, a1_pclk_pdata);
>>>>>> +AUDIO_GATE(audio_loopback, AUDIO_CLK_GATE_EN0, 4, a1_pclk_pdata);
>>>>>> +AUDIO_GATE(audio_tdmout_a, AUDIO_CLK_GATE_EN0, 5, a1_pclk_pdata);
>>>>>> +AUDIO_GATE(audio_tdmout_b, AUDIO_CLK_GATE_EN0, 6, a1_pclk_pdata);
>>>>>> +AUDIO_GATE(audio_frddr_a, AUDIO_CLK_GATE_EN0, 7, a1_pclk_pdata);
>>>>>> +AUDIO_GATE(audio_frddr_b, AUDIO_CLK_GATE_EN0, 8, a1_pclk_pdata);
>>>>>> +AUDIO_GATE(audio_toddr_a, AUDIO_CLK_GATE_EN0, 9, a1_pclk_pdata);
>>>>>> +AUDIO_GATE(audio_toddr_b, AUDIO_CLK_GATE_EN0, 10, a1_pclk_pdata);
>>>>>> +AUDIO_GATE(audio_spdifin, AUDIO_CLK_GATE_EN0, 11, a1_pclk_pdata);
>>>>>> +AUDIO_GATE(audio_resample, AUDIO_CLK_GATE_EN0, 12, a1_pclk_pdata);
>>>>>> +AUDIO_GATE(audio_eqdrc, AUDIO_CLK_GATE_EN0, 13, a1_pclk_pdata);
>>>>>> +AUDIO_GATE(audio_audiolocker, AUDIO_CLK_GATE_EN0, 14, a1_pclk_pdata);
>>>>> This is what I mean by redundant parameter ^
>>>>>
>>>>
>>>> Yep. I could define something like AUDIO_PCLK_GATE().
>>>>
>>>>>> +
>>>>>> +AUDIO_GATE(audio2_ddr_arb, AUDIO2_CLK_GATE_EN0, 0, a1_pclk_pdata);
>>>>>> +AUDIO_GATE(audio2_pdm, AUDIO2_CLK_GATE_EN0, 1, a1_pclk_pdata);
>>>>>> +AUDIO_GATE(audio2_tdmin_vad, AUDIO2_CLK_GATE_EN0, 2, a1_pclk_pdata);
>>>>>> +AUDIO_GATE(audio2_toddr_vad, AUDIO2_CLK_GATE_EN0, 3, a1_pclk_pdata);
>>>>>> +AUDIO_GATE(audio2_vad, AUDIO2_CLK_GATE_EN0, 4, a1_pclk_pdata);
>>>>>> +AUDIO_GATE(audio2_audiotop, AUDIO2_CLK_GATE_EN0, 7, a1_pclk_pdata);
>>>>>> +
>>>>>> +static const struct clk_parent_data a1_mst_pdata[] = {
>>>>>> + { .fw_name = "dds_in" },
>>>>>> + { .fw_name = "fclk_div2" },
>>>>>> + { .fw_name = "fclk_div3" },
>>>>>> + { .fw_name = "hifi_pll" },
>>>>>> + { .fw_name = "xtal" },
>>>>>> +};
>>>>>> +
>>>>>> +#define AUDIO_MST_MCLK(_name, _reg) \
>>>>>> + AUDIO_MUX(_name##_mux, (_reg), 0x7, 24, a1_mst_pdata); \
>>>>>> + AUDIO_DIV(_name##_div, (_reg), 0, 16, \
>>>>>> + AUDIO_PDATA(_name##_mux)); \
>>>>>> + AUDIO_GATE(_name, (_reg), 31, AUDIO_PDATA(_name##_div))
>>>>>> +
>>>>>> +AUDIO_MST_MCLK(audio_mst_a_mclk, AUDIO_MCLK_A_CTRL);
>>>>>> +AUDIO_MST_MCLK(audio_mst_b_mclk, AUDIO_MCLK_B_CTRL);
>>>>>> +AUDIO_MST_MCLK(audio_mst_c_mclk, AUDIO_MCLK_C_CTRL);
>>>>>> +AUDIO_MST_MCLK(audio_mst_d_mclk, AUDIO_MCLK_D_CTRL);
>>>>>> +AUDIO_MST_MCLK(audio_spdifin_clk, AUDIO_CLK_SPDIFIN_CTRL);
>>>>>> +AUDIO_MST_MCLK(audio_eqdrc_clk, AUDIO_CLK_EQDRC_CTRL);
>>>>>> +
>>>>>> +AUDIO_MUX(audio_resample_clk_mux, AUDIO_CLK_RESAMPLE_CTRL, 0xf, 24,
>>>>>> + a1_mst_pdata);
>>>>>> +AUDIO_DIV(audio_resample_clk_div, AUDIO_CLK_RESAMPLE_CTRL, 0, 8,
>>>>>> + AUDIO_PDATA(audio_resample_clk_mux));
>>>>>> +AUDIO_GATE(audio_resample_clk, AUDIO_CLK_RESAMPLE_CTRL, 31,
>>>>>> + AUDIO_PDATA(audio_resample_clk_div));
>>>>>> +
>>>>>> +AUDIO_MUX(audio_locker_in_clk_mux, AUDIO_CLK_LOCKER_CTRL, 0xf, 8,
>>>>>> + a1_mst_pdata);
>>>>>> +AUDIO_DIV(audio_locker_in_clk_div, AUDIO_CLK_LOCKER_CTRL, 0, 8,
>>>>>> + AUDIO_PDATA(audio_locker_in_clk_mux));
>>>>>> +AUDIO_GATE(audio_locker_in_clk, AUDIO_CLK_LOCKER_CTRL, 15,
>>>>>> + AUDIO_PDATA(audio_locker_in_clk_div));
>>>>>> +
>>>>>> +AUDIO_MUX(audio_locker_out_clk_mux, AUDIO_CLK_LOCKER_CTRL, 0xf, 24,
>>>>>> + a1_mst_pdata);
>>>>>> +AUDIO_DIV(audio_locker_out_clk_div, AUDIO_CLK_LOCKER_CTRL, 16, 8,
>>>>>> + AUDIO_PDATA(audio_locker_out_clk_mux));
>>>>>> +AUDIO_GATE(audio_locker_out_clk, AUDIO_CLK_LOCKER_CTRL, 31,
>>>>>> + AUDIO_PDATA(audio_locker_out_clk_div));
>>>>>> +
>>>>>> +AUDIO_MST_MCLK(audio2_vad_mclk, AUDIO2_MCLK_VAD_CTRL);
>>>>>> +AUDIO_MST_MCLK(audio2_vad_clk, AUDIO2_CLK_VAD_CTRL);
>>>>>> +AUDIO_MST_MCLK(audio2_pdm_dclk, AUDIO2_CLK_PDMIN_CTRL0);
>>>>>> +AUDIO_MST_MCLK(audio2_pdm_sysclk, AUDIO2_CLK_PDMIN_CTRL1);
>>>>>> +
>>>>>> +#define AUDIO_MST_SCLK(_name, _reg0, _reg1, _pdata) \
>>>>>> + AUDIO_GATE(_name##_pre_en, (_reg0), 31, (_pdata)); \
>>>>>> + AUDIO_SCLK_DIV(_name##_div, (_reg0), 20, 10, 0, 0, \
>>>>>> + AUDIO_PDATA(_name##_pre_en), true); \
>>>>>> + AUDIO_GATE(_name##_post_en, (_reg0), 30, \
>>>>>> + AUDIO_PDATA(_name##_div)); \
>>>>>> + AUDIO_TRIPHASE(_name, (_reg1), 1, 0, 2, 4, \
>>>>>> + AUDIO_PDATA(_name##_post_en))
>>>>>> +
>>>>>
>>>>> Again, I'm not a fan of this many levels of macro. I can live with it
>>>>> but certainly don't want the burden of reviewing and maintaining for
>>>>> clock driver. AXG / G12 and A1 are obviously closely related, so make it common.
>>>>>
>>>>>> +#define AUDIO_MST_LRCLK(_name, _reg0, _reg1, _pdata) \
>>>>>> + AUDIO_SCLK_DIV(_name##_div, (_reg0), 0, 10, 10, 10, \
>>>>>> + (_pdata), false); \
>>>>>> + AUDIO_TRIPHASE(_name, (_reg1), 1, 1, 3, 5, \
>>>>>> + AUDIO_PDATA(_name##_div))
>>>>>> +
>>>>>> +AUDIO_MST_SCLK(audio_mst_a_sclk, AUDIO_MST_A_SCLK_CTRL0, AUDIO_MST_A_SCLK_CTRL1,
>>>>>> + AUDIO_PDATA(audio_mst_a_mclk));
>>>>>> +AUDIO_MST_SCLK(audio_mst_b_sclk, AUDIO_MST_B_SCLK_CTRL0, AUDIO_MST_B_SCLK_CTRL1,
>>>>>> + AUDIO_PDATA(audio_mst_b_mclk));
>>>>>> +AUDIO_MST_SCLK(audio_mst_c_sclk, AUDIO_MST_C_SCLK_CTRL0, AUDIO_MST_C_SCLK_CTRL1,
>>>>>> + AUDIO_PDATA(audio_mst_c_mclk));
>>>>>> +AUDIO_MST_SCLK(audio_mst_d_sclk, AUDIO_MST_D_SCLK_CTRL0, AUDIO_MST_D_SCLK_CTRL1,
>>>>>> + AUDIO_PDATA(audio_mst_d_mclk));
>>>>>> +
>>>>>> +AUDIO_MST_LRCLK(audio_mst_a_lrclk, AUDIO_MST_A_SCLK_CTRL0, AUDIO_MST_A_SCLK_CTRL1,
>>>>>> + AUDIO_PDATA(audio_mst_a_sclk_post_en));
>>>>>> +AUDIO_MST_LRCLK(audio_mst_b_lrclk, AUDIO_MST_B_SCLK_CTRL0, AUDIO_MST_B_SCLK_CTRL1,
>>>>>> + AUDIO_PDATA(audio_mst_b_sclk_post_en));
>>>>>> +AUDIO_MST_LRCLK(audio_mst_c_lrclk, AUDIO_MST_C_SCLK_CTRL0, AUDIO_MST_C_SCLK_CTRL1,
>>>>>> + AUDIO_PDATA(audio_mst_c_sclk_post_en));
>>>>>> +AUDIO_MST_LRCLK(audio_mst_d_lrclk, AUDIO_MST_D_SCLK_CTRL0, AUDIO_MST_D_SCLK_CTRL1,
>>>>>> + AUDIO_PDATA(audio_mst_d_sclk_post_en));
>>>>>> +
>>>>>> +static const struct clk_parent_data a1_mst_sclk_pdata[] = {
>>>>>> + { .hw = &audio_mst_a_sclk.hw },
>>>>>> + { .hw = &audio_mst_b_sclk.hw },
>>>>>> + { .hw = &audio_mst_c_sclk.hw },
>>>>>> + { .hw = &audio_mst_d_sclk.hw },
>>>>>> + { .fw_name = "slv_sclk0" },
>>>>>> + { .fw_name = "slv_sclk1" },
>>>>>> + { .fw_name = "slv_sclk2" },
>>>>>> + { .fw_name = "slv_sclk3" },
>>>>>> + { .fw_name = "slv_sclk4" },
>>>>>> + { .fw_name = "slv_sclk5" },
>>>>>> + { .fw_name = "slv_sclk6" },
>>>>>> + { .fw_name = "slv_sclk7" },
>>>>>> + { .fw_name = "slv_sclk8" },
>>>>>> + { .fw_name = "slv_sclk9" },
>>>>>> +};
>>>>>> +
>>>>>> +static const struct clk_parent_data a1_mst_lrclk_pdata[] = {
>>>>>> + { .hw = &audio_mst_a_lrclk.hw },
>>>>>> + { .hw = &audio_mst_b_lrclk.hw },
>>>>>> + { .hw = &audio_mst_c_lrclk.hw },
>>>>>> + { .hw = &audio_mst_d_lrclk.hw },
>>>>>> + { .fw_name = "slv_lrclk0" },
>>>>>> + { .fw_name = "slv_lrclk1" },
>>>>>> + { .fw_name = "slv_lrclk2" },
>>>>>> + { .fw_name = "slv_lrclk3" },
>>>>>> + { .fw_name = "slv_lrclk4" },
>>>>>> + { .fw_name = "slv_lrclk5" },
>>>>>> + { .fw_name = "slv_lrclk6" },
>>>>>> + { .fw_name = "slv_lrclk7" },
>>>>>> + { .fw_name = "slv_lrclk8" },
>>>>>> + { .fw_name = "slv_lrclk9" },
>>>>>> +};
>>>>>> +
>>>>>> +#define AUDIO_TDM_SCLK(_name, _reg) \
>>>>>> + AUDIO_MUX(_name##_mux, (_reg), 0xf, 24, a1_mst_sclk_pdata); \
>>>>>> + AUDIO_GATE(_name##_pre_en, (_reg), 31, \
>>>>>> + AUDIO_PDATA(_name##_mux)); \
>>>>>> + AUDIO_GATE(_name##_post_en, (_reg), 30, \
>>>>>> + AUDIO_PDATA(_name##_pre_en)); \
>>>>>> + AUDIO_SCLK_WS(_name, (_reg), 1, 29, 28, \
>>>>>> + AUDIO_PDATA(_name##_post_en))
>>>>>> +
>>>>>> +#define AUDIO_TDM_LRCLK(_name, _reg) \
>>>>>> + AUDIO_MUX(_name, (_reg), 0xf, 20, a1_mst_lrclk_pdata)
>>>>>> +
>>>>>> +AUDIO_TDM_SCLK(audio_tdmin_a_sclk, AUDIO_CLK_TDMIN_A_CTRL);
>>>>>> +AUDIO_TDM_SCLK(audio_tdmin_b_sclk, AUDIO_CLK_TDMIN_B_CTRL);
>>>>>> +AUDIO_TDM_SCLK(audio_tdmin_lb_sclk, AUDIO_CLK_TDMIN_LB_CTRL);
>>>>>> +AUDIO_TDM_SCLK(audio_tdmout_a_sclk, AUDIO_CLK_TDMOUT_A_CTRL);
>>>>>> +AUDIO_TDM_SCLK(audio_tdmout_b_sclk, AUDIO_CLK_TDMOUT_B_CTRL);
>>>>>> +
>>>>>> +AUDIO_TDM_LRCLK(audio_tdmin_a_lrclk, AUDIO_CLK_TDMIN_A_CTRL);
>>>>>> +AUDIO_TDM_LRCLK(audio_tdmin_b_lrclk, AUDIO_CLK_TDMIN_B_CTRL);
>>>>>> +AUDIO_TDM_LRCLK(audio_tdmin_lb_lrclk, AUDIO_CLK_TDMIN_LB_CTRL);
>>>>>> +AUDIO_TDM_LRCLK(audio_tdmout_a_lrclk, AUDIO_CLK_TDMOUT_A_CTRL);
>>>>>> +AUDIO_TDM_LRCLK(audio_tdmout_b_lrclk, AUDIO_CLK_TDMOUT_B_CTRL);
>>>>>> +
>>>>>> +static struct clk_hw *a1_audio_hw_clks[] = {
>>>>>> + [AUD_CLKID_DDR_ARB] = &audio_ddr_arb.hw,
>>>>>> + [AUD_CLKID_TDMIN_A] = &audio_tdmin_a.hw,
>>>>>> + [AUD_CLKID_TDMIN_B] = &audio_tdmin_b.hw,
>>>>>> + [AUD_CLKID_TDMIN_LB] = &audio_tdmin_lb.hw,
>>>>>> + [AUD_CLKID_LOOPBACK] = &audio_loopback.hw,
>>>>>> + [AUD_CLKID_TDMOUT_A] = &audio_tdmout_a.hw,
>>>>>> + [AUD_CLKID_TDMOUT_B] = &audio_tdmout_b.hw,
>>>>>> + [AUD_CLKID_FRDDR_A] = &audio_frddr_a.hw,
>>>>>> + [AUD_CLKID_FRDDR_B] = &audio_frddr_b.hw,
>>>>>> + [AUD_CLKID_TODDR_A] = &audio_toddr_a.hw,
>>>>>> + [AUD_CLKID_TODDR_B] = &audio_toddr_b.hw,
>>>>>> + [AUD_CLKID_SPDIFIN] = &audio_spdifin.hw,
>>>>>> + [AUD_CLKID_RESAMPLE] = &audio_resample.hw,
>>>>>> + [AUD_CLKID_EQDRC] = &audio_eqdrc.hw,
>>>>>> + [AUD_CLKID_LOCKER] = &audio_audiolocker.hw,
>>>>>> + [AUD_CLKID_MST_A_MCLK_SEL] = &audio_mst_a_mclk_mux.hw,
>>>>>> + [AUD_CLKID_MST_A_MCLK_DIV] = &audio_mst_a_mclk_div.hw,
>>>>>> + [AUD_CLKID_MST_A_MCLK] = &audio_mst_a_mclk.hw,
>>>>>> + [AUD_CLKID_MST_B_MCLK_SEL] = &audio_mst_b_mclk_mux.hw,
>>>>>> + [AUD_CLKID_MST_B_MCLK_DIV] = &audio_mst_b_mclk_div.hw,
>>>>>> + [AUD_CLKID_MST_B_MCLK] = &audio_mst_b_mclk.hw,
>>>>>> + [AUD_CLKID_MST_C_MCLK_SEL] = &audio_mst_c_mclk_mux.hw,
>>>>>> + [AUD_CLKID_MST_C_MCLK_DIV] = &audio_mst_c_mclk_div.hw,
>>>>>> + [AUD_CLKID_MST_C_MCLK] = &audio_mst_c_mclk.hw,
>>>>>> + [AUD_CLKID_MST_D_MCLK_SEL] = &audio_mst_d_mclk_mux.hw,
>>>>>> + [AUD_CLKID_MST_D_MCLK_DIV] = &audio_mst_d_mclk_div.hw,
>>>>>> + [AUD_CLKID_MST_D_MCLK] = &audio_mst_d_mclk.hw,
>>>>>> + [AUD_CLKID_RESAMPLE_CLK_SEL] = &audio_resample_clk_mux.hw,
>>>>>> + [AUD_CLKID_RESAMPLE_CLK_DIV] = &audio_resample_clk_div.hw,
>>>>>> + [AUD_CLKID_RESAMPLE_CLK] = &audio_resample_clk.hw,
>>>>>> + [AUD_CLKID_LOCKER_IN_CLK_SEL] = &audio_locker_in_clk_mux.hw,
>>>>>> + [AUD_CLKID_LOCKER_IN_CLK_DIV] = &audio_locker_in_clk_div.hw,
>>>>>> + [AUD_CLKID_LOCKER_IN_CLK] = &audio_locker_in_clk.hw,
>>>>>> + [AUD_CLKID_LOCKER_OUT_CLK_SEL] = &audio_locker_out_clk_mux.hw,
>>>>>> + [AUD_CLKID_LOCKER_OUT_CLK_DIV] = &audio_locker_out_clk_div.hw,
>>>>>> + [AUD_CLKID_LOCKER_OUT_CLK] = &audio_locker_out_clk.hw,
>>>>>> + [AUD_CLKID_SPDIFIN_CLK_SEL] = &audio_spdifin_clk_mux.hw,
>>>>>> + [AUD_CLKID_SPDIFIN_CLK_DIV] = &audio_spdifin_clk_div.hw,
>>>>>> + [AUD_CLKID_SPDIFIN_CLK] = &audio_spdifin_clk.hw,
>>>>>> + [AUD_CLKID_EQDRC_CLK_SEL] = &audio_eqdrc_clk_mux.hw,
>>>>>> + [AUD_CLKID_EQDRC_CLK_DIV] = &audio_eqdrc_clk_div.hw,
>>>>>> + [AUD_CLKID_EQDRC_CLK] = &audio_eqdrc_clk.hw,
>>>>>> + [AUD_CLKID_MST_A_SCLK_PRE_EN] = &audio_mst_a_sclk_pre_en.hw,
>>>>>> + [AUD_CLKID_MST_A_SCLK_DIV] = &audio_mst_a_sclk_div.hw,
>>>>>> + [AUD_CLKID_MST_A_SCLK_POST_EN] = &audio_mst_a_sclk_post_en.hw,
>>>>>> + [AUD_CLKID_MST_A_SCLK] = &audio_mst_a_sclk.hw,
>>>>>> + [AUD_CLKID_MST_B_SCLK_PRE_EN] = &audio_mst_b_sclk_pre_en.hw,
>>>>>> + [AUD_CLKID_MST_B_SCLK_DIV] = &audio_mst_b_sclk_div.hw,
>>>>>> + [AUD_CLKID_MST_B_SCLK_POST_EN] = &audio_mst_b_sclk_post_en.hw,
>>>>>> + [AUD_CLKID_MST_B_SCLK] = &audio_mst_b_sclk.hw,
>>>>>> + [AUD_CLKID_MST_C_SCLK_PRE_EN] = &audio_mst_c_sclk_pre_en.hw,
>>>>>> + [AUD_CLKID_MST_C_SCLK_DIV] = &audio_mst_c_sclk_div.hw,
>>>>>> + [AUD_CLKID_MST_C_SCLK_POST_EN] = &audio_mst_c_sclk_post_en.hw,
>>>>>> + [AUD_CLKID_MST_C_SCLK] = &audio_mst_c_sclk.hw,
>>>>>> + [AUD_CLKID_MST_D_SCLK_PRE_EN] = &audio_mst_d_sclk_pre_en.hw,
>>>>>> + [AUD_CLKID_MST_D_SCLK_DIV] = &audio_mst_d_sclk_div.hw,
>>>>>> + [AUD_CLKID_MST_D_SCLK_POST_EN] = &audio_mst_d_sclk_post_en.hw,
>>>>>> + [AUD_CLKID_MST_D_SCLK] = &audio_mst_d_sclk.hw,
>>>>>> + [AUD_CLKID_MST_A_LRCLK_DIV] = &audio_mst_a_lrclk_div.hw,
>>>>>> + [AUD_CLKID_MST_A_LRCLK] = &audio_mst_a_lrclk.hw,
>>>>>> + [AUD_CLKID_MST_B_LRCLK_DIV] = &audio_mst_b_lrclk_div.hw,
>>>>>> + [AUD_CLKID_MST_B_LRCLK] = &audio_mst_b_lrclk.hw,
>>>>>> + [AUD_CLKID_MST_C_LRCLK_DIV] = &audio_mst_c_lrclk_div.hw,
>>>>>> + [AUD_CLKID_MST_C_LRCLK] = &audio_mst_c_lrclk.hw,
>>>>>> + [AUD_CLKID_MST_D_LRCLK_DIV] = &audio_mst_d_lrclk_div.hw,
>>>>>> + [AUD_CLKID_MST_D_LRCLK] = &audio_mst_d_lrclk.hw,
>>>>>> + [AUD_CLKID_TDMIN_A_SCLK_SEL] = &audio_tdmin_a_sclk_mux.hw,
>>>>>> + [AUD_CLKID_TDMIN_A_SCLK_PRE_EN] = &audio_tdmin_a_sclk_pre_en.hw,
>>>>>> + [AUD_CLKID_TDMIN_A_SCLK_POST_EN] = &audio_tdmin_a_sclk_post_en.hw,
>>>>>> + [AUD_CLKID_TDMIN_A_SCLK] = &audio_tdmin_a_sclk.hw,
>>>>>> + [AUD_CLKID_TDMIN_A_LRCLK] = &audio_tdmin_a_lrclk.hw,
>>>>>> + [AUD_CLKID_TDMIN_B_SCLK_SEL] = &audio_tdmin_b_sclk_mux.hw,
>>>>>> + [AUD_CLKID_TDMIN_B_SCLK_PRE_EN] = &audio_tdmin_b_sclk_pre_en.hw,
>>>>>> + [AUD_CLKID_TDMIN_B_SCLK_POST_EN] = &audio_tdmin_b_sclk_post_en.hw,
>>>>>> + [AUD_CLKID_TDMIN_B_SCLK] = &audio_tdmin_b_sclk.hw,
>>>>>> + [AUD_CLKID_TDMIN_B_LRCLK] = &audio_tdmin_b_lrclk.hw,
>>>>>> + [AUD_CLKID_TDMIN_LB_SCLK_SEL] = &audio_tdmin_lb_sclk_mux.hw,
>>>>>> + [AUD_CLKID_TDMIN_LB_SCLK_PRE_EN] = &audio_tdmin_lb_sclk_pre_en.hw,
>>>>>> + [AUD_CLKID_TDMIN_LB_SCLK_POST_EN] = &audio_tdmin_lb_sclk_post_en.hw,
>>>>>> + [AUD_CLKID_TDMIN_LB_SCLK] = &audio_tdmin_lb_sclk.hw,
>>>>>> + [AUD_CLKID_TDMIN_LB_LRCLK] = &audio_tdmin_lb_lrclk.hw,
>>>>>> + [AUD_CLKID_TDMOUT_A_SCLK_SEL] = &audio_tdmout_a_sclk_mux.hw,
>>>>>> + [AUD_CLKID_TDMOUT_A_SCLK_PRE_EN] = &audio_tdmout_a_sclk_pre_en.hw,
>>>>>> + [AUD_CLKID_TDMOUT_A_SCLK_POST_EN] = &audio_tdmout_a_sclk_post_en.hw,
>>>>>> + [AUD_CLKID_TDMOUT_A_SCLK] = &audio_tdmout_a_sclk.hw,
>>>>>> + [AUD_CLKID_TDMOUT_A_LRCLK] = &audio_tdmout_a_lrclk.hw,
>>>>>> + [AUD_CLKID_TDMOUT_B_SCLK_SEL] = &audio_tdmout_b_sclk_mux.hw,
>>>>>> + [AUD_CLKID_TDMOUT_B_SCLK_PRE_EN] = &audio_tdmout_b_sclk_pre_en.hw,
>>>>>> + [AUD_CLKID_TDMOUT_B_SCLK_POST_EN] = &audio_tdmout_b_sclk_post_en.hw,
>>>>>> + [AUD_CLKID_TDMOUT_B_SCLK] = &audio_tdmout_b_sclk.hw,
>>>>>> + [AUD_CLKID_TDMOUT_B_LRCLK] = &audio_tdmout_b_lrclk.hw,
>>>>>> +
>>>>>> + [AUD2_CLKID_DDR_ARB] = &audio2_ddr_arb.hw,
>>>>>> + [AUD2_CLKID_PDM] = &audio2_pdm.hw,
>>>>>> + [AUD2_CLKID_TDMIN_VAD] = &audio2_tdmin_vad.hw,
>>>>>> + [AUD2_CLKID_TODDR_VAD] = &audio2_toddr_vad.hw,
>>>>>> + [AUD2_CLKID_VAD] = &audio2_vad.hw,
>>>>>> + [AUD2_CLKID_AUDIOTOP] = &audio2_audiotop.hw,
>>>>>> + [AUD2_CLKID_VAD_MCLK_SEL] = &audio2_vad_mclk_mux.hw,
>>>>>> + [AUD2_CLKID_VAD_MCLK_DIV] = &audio2_vad_mclk_div.hw,
>>>>>> + [AUD2_CLKID_VAD_MCLK] = &audio2_vad_mclk.hw,
>>>>>> + [AUD2_CLKID_VAD_CLK_SEL] = &audio2_vad_clk_mux.hw,
>>>>>> + [AUD2_CLKID_VAD_CLK_DIV] = &audio2_vad_clk_div.hw,
>>>>>> + [AUD2_CLKID_VAD_CLK] = &audio2_vad_clk.hw,
>>>>>> + [AUD2_CLKID_PDM_DCLK_SEL] = &audio2_pdm_dclk_mux.hw,
>>>>>> + [AUD2_CLKID_PDM_DCLK_DIV] = &audio2_pdm_dclk_div.hw,
>>>>>> + [AUD2_CLKID_PDM_DCLK] = &audio2_pdm_dclk.hw,
>>>>>> + [AUD2_CLKID_PDM_SYSCLK_SEL] = &audio2_pdm_sysclk_mux.hw,
>>>>>> + [AUD2_CLKID_PDM_SYSCLK_DIV] = &audio2_pdm_sysclk_div.hw,
>>>>>> + [AUD2_CLKID_PDM_SYSCLK] = &audio2_pdm_sysclk.hw,
>>>>>> +};
>>>>>> +
>>>>>> +static struct meson_clk_hw_data a1_audio_clks = {
>>>>>> + .hws = a1_audio_hw_clks,
>>>>>> + .num = ARRAY_SIZE(a1_audio_hw_clks),
>>>>>> +};
>>>>>> +
>>>>>> +static struct regmap *a1_audio_map(struct platform_device *pdev,
>>>>>> + unsigned int index)
>>>>>> +{
>>>>>> + char name[32];
>>>>>> + const struct regmap_config cfg = {
>>>>>> + .reg_bits = 32,
>>>>>> + .val_bits = 32,
>>>>>> + .reg_stride = 4,
>>>>>> + .name = name,
>>>>>
>>>>> Not necessary
>>>>>
>>>>
>>>> This implementation uses two regmaps, and this field allow to avoid
>>>> errors like this:
>>>>
>>>> [ 0.145530] debugfs: Directory 'fe050000.audio-clock-controller' with
>>>> parent 'regmap' already present!
>>>>
>>>>>> + };
>>>>>> + void __iomem *base;
>>>>>> +
>>>>>> + base = devm_platform_ioremap_resource(pdev, index);
>>>>>> + if (IS_ERR(base))
>>>>>> + return base;
>>>>>> +
>>>>>> + scnprintf(name, sizeof(name), "%d", index);
>>>>>> + return devm_regmap_init_mmio(&pdev->dev, base, &cfg);
>>>>>> +}
>>>>>
>>>>> That is overengineered. Please keep it simple. Declare the regmap_config
>>>>> as static const global, and do it like axg-audio please.
>>>>>
>>>>
>>>> This only reason why it is not "static const" because I need to set
>>>> unique name for each regmap.
>>>>
>>>>>> +
>>>>>> +static int a1_register_clk(struct platform_device *pdev,
>>>>>> + struct regmap *map0, struct regmap *map1,
>>>>>> + struct clk_hw *hw)
>>>>>> +{
>>>>>> + struct clk_regmap *clk = container_of(hw, struct clk_regmap, hw);
>>>>>> +
>>>>>> + if (!hw)
>>>>>> + return 0;
>>>>>> +
>>>>>> + switch ((unsigned long)clk->map) {
>>>>>> + case AUDIO_RANGE_0:
>>>>>> + clk->map = map0;
>>>>>> + break;
>>>>>> + case AUDIO_RANGE_1:
>>>>>> + clk->map = map1;
>>>>>> + break;
>>>>>
>>>>> ... fishy
>>>>>
>>>>>> + default:
>>>>>> + WARN_ON(1);
>>>>>> + return -EINVAL;
>>>>>> + }
>>>>>> +
>>>>>> + return devm_clk_hw_register(&pdev->dev, hw);
>>>>>> +}
>>>>>> +
>>>>>> +static int a1_audio_clkc_probe(struct platform_device *pdev)
>>>>>> +{
>>>>>> + struct regmap *map0, *map1;
>>>>>> + struct clk *clk;
>>>>>> + unsigned int i;
>>>>>> + int ret;
>>>>>> +
>>>>>> + clk = devm_clk_get_enabled(&pdev->dev, "pclk");
>>>>>> + if (WARN_ON(IS_ERR(clk)))
>>>>>> + return PTR_ERR(clk);
>>>>>> +
>>>>>> + map0 = a1_audio_map(pdev, 0);
>>>>>> + if (IS_ERR(map0))
>>>>>> + return PTR_ERR(map0);
>>>>>> +
>>>>>> + map1 = a1_audio_map(pdev, 1);
>>>>>> + if (IS_ERR(map1))
>>>>>> + return PTR_ERR(map1);
>>>>>
>>>>> No - Looks to me you just have two clock controllers you are trying
>>>>> force into one.
>>>>>
>>>>
>>>> See the begining.
>>>>
>>>>>> +
>>>>>> + /*
>>>>>> + * Register and enable AUD2_CLKID_AUDIOTOP clock first. Unless
>>>>>> + * it is enabled any read/write to 'map0' hangs the CPU.
>>>>>> + */
>>>>>> +
>>>>>> + ret = a1_register_clk(pdev, map0, map1,
>>>>>> + a1_audio_clks.hws[AUD2_CLKID_AUDIOTOP]);
>>>>>> + if (ret)
>>>>>> + return ret;
>>>>>> +
>>>>>> + ret = clk_prepare_enable(a1_audio_clks.hws[AUD2_CLKID_AUDIOTOP]->clk);
>>>>>> + if (ret)
>>>>>> + return ret;
>>>>>
>>>>> Again, this shows 2 devices. The one related to your 'map0' should
>>>>> request AUD2_CLKID_AUDIOTOP as input and enable it right away.
>>>>>
>>>>
>>>> See the begining.
>>>>
>>>>>> +
>>>>>> + for (i = 0; i < a1_audio_clks.num; i++) {
>>>>>> + if (i == AUD2_CLKID_AUDIOTOP)
>>>>>> + continue;
>>>>>> +
>>>>>> + ret = a1_register_clk(pdev, map0, map1, a1_audio_clks.hws[i]);
>>>>>> + if (ret)
>>>>>> + return ret;
>>>>>> + }
>>>>>> +
>>>>>> + ret = devm_of_clk_add_hw_provider(&pdev->dev, meson_clk_hw_get,
>>>>>> + &a1_audio_clks);
>>>>>> + if (ret)
>>>>>> + return ret;
>>>>>> +
>>>>>> + BUILD_BUG_ON((unsigned long)AUDIO_REG_MAP(AUDIO_SW_RESET0) !=
>>>>>> + AUDIO_RANGE_0);
>>>>>
>>>>> Why is that necessary ?
>>>>>
>>>>
>>>> A little paranoia. Here AUDIO_SW_RESET0 is handled as map0's register,
>>>> and I want to assert it.
>>>>
>>>>>> + return meson_audio_rstc_register(&pdev->dev, map0,
>>>>>> + AUDIO_REG_OFFSET(AUDIO_SW_RESET0), 32);
>>>>>> +}
>>>>>> +
>>>>>> +static const struct of_device_id a1_audio_clkc_match_table[] = {
>>>>>> + { .compatible = "amlogic,a1-audio-clkc", },
>>>>>> + {}
>>>>>> +};
>>>>>> +MODULE_DEVICE_TABLE(of, a1_audio_clkc_match_table);
>>>>>> +
>>>>>> +static struct platform_driver a1_audio_clkc_driver = {
>>>>>> + .probe = a1_audio_clkc_probe,
>>>>>> + .driver = {
>>>>>> + .name = "a1-audio-clkc",
>>>>>> + .of_match_table = a1_audio_clkc_match_table,
>>>>>> + },
>>>>>> +};
>>>>>> +module_platform_driver(a1_audio_clkc_driver);
>>>>>> +
>>>>>> +MODULE_DESCRIPTION("Amlogic A1 Audio Clock driver");
>>>>>> +MODULE_AUTHOR("Jan Dakinevich <jan.dakinevich@salutedevices.com>");
>>>>>> +MODULE_LICENSE("GPL");
>>>>>> diff --git a/drivers/clk/meson/a1-audio.h b/drivers/clk/meson/a1-audio.h
>>>>>> new file mode 100644
>>>>>> index 000000000000..f994e87276cd
>>>>>> --- /dev/null
>>>>>> +++ b/drivers/clk/meson/a1-audio.h
>>>>>> @@ -0,0 +1,58 @@
>>>>>> +/* SPDX-License-Identifier: (GPL-2.0 OR MIT) */
>>>>>> +/*
>>>>>> + * Copyright (c) 2024, SaluteDevices. All Rights Reserved.
>>>>>> + *
>>>>>> + * Author: Jan Dakinevich <jan.dakinevich@salutedevices.com>
>>>>>> + */
>>>>>> +
>>>>>> +#ifndef __A1_AUDIO_H
>>>>>> +#define __A1_AUDIO_H
>>>>>> +
>>>>>> +#define AUDIO_RANGE_0 0xa
>>>>>> +#define AUDIO_RANGE_1 0xb
>>>>>> +#define AUDIO_RANGE_SHIFT 16
>>>>>> +
>>>>>> +#define AUDIO_REG(_range, _offset) \
>>>>>> + (((_range) << AUDIO_RANGE_SHIFT) + (_offset))
>>>>>> +
>>>>>> +#define AUDIO_REG_OFFSET(_reg) \
>>>>>> + ((_reg) & ((1 << AUDIO_RANGE_SHIFT) - 1))
>>>>>> +
>>>>>> +#define AUDIO_REG_MAP(_reg) \
>>>>>> + ((void *)((_reg) >> AUDIO_RANGE_SHIFT))
>>>>>
>>>>> That is seriouly overengineered.
>>>>> The following are offset. Just write what they are.
>>>>>
>>>>
>>>> This is all in order to keep range's identifier together with offset and
>>>> then use it to store the identifier in clk_regmaps.
>>>>
>>>>> There is not reason to put that into a header. It is only going to be
>>>>> used by a single driver.
>>>>>>> +
>>>>>> +#define AUDIO_CLK_GATE_EN0 AUDIO_REG(AUDIO_RANGE_0, 0x000)
>>>>>> +#define AUDIO_MCLK_A_CTRL AUDIO_REG(AUDIO_RANGE_0, 0x008)
>>>>>> +#define AUDIO_MCLK_B_CTRL AUDIO_REG(AUDIO_RANGE_0, 0x00c)
>>>>>> +#define AUDIO_MCLK_C_CTRL AUDIO_REG(AUDIO_RANGE_0, 0x010)
>>>>>> +#define AUDIO_MCLK_D_CTRL AUDIO_REG(AUDIO_RANGE_0, 0x014)
>>>>>> +#define AUDIO_MCLK_E_CTRL AUDIO_REG(AUDIO_RANGE_0, 0x018)
>>>>>> +#define AUDIO_MCLK_F_CTRL AUDIO_REG(AUDIO_RANGE_0, 0x01c)
>>>>>> +#define AUDIO_SW_RESET0 AUDIO_REG(AUDIO_RANGE_0, 0x028)
>>>>>> +#define AUDIO_MST_A_SCLK_CTRL0 AUDIO_REG(AUDIO_RANGE_0, 0x040)
>>>>>> +#define AUDIO_MST_A_SCLK_CTRL1 AUDIO_REG(AUDIO_RANGE_0, 0x044)
>>>>>> +#define AUDIO_MST_B_SCLK_CTRL0 AUDIO_REG(AUDIO_RANGE_0, 0x048)
>>>>>> +#define AUDIO_MST_B_SCLK_CTRL1 AUDIO_REG(AUDIO_RANGE_0, 0x04c)
>>>>>> +#define AUDIO_MST_C_SCLK_CTRL0 AUDIO_REG(AUDIO_RANGE_0, 0x050)
>>>>>> +#define AUDIO_MST_C_SCLK_CTRL1 AUDIO_REG(AUDIO_RANGE_0, 0x054)
>>>>>> +#define AUDIO_MST_D_SCLK_CTRL0 AUDIO_REG(AUDIO_RANGE_0, 0x058)
>>>>>> +#define AUDIO_MST_D_SCLK_CTRL1 AUDIO_REG(AUDIO_RANGE_0, 0x05c)
>>>>>> +#define AUDIO_CLK_TDMIN_A_CTRL AUDIO_REG(AUDIO_RANGE_0, 0x080)
>>>>>> +#define AUDIO_CLK_TDMIN_B_CTRL AUDIO_REG(AUDIO_RANGE_0, 0x084)
>>>>>> +#define AUDIO_CLK_TDMIN_LB_CTRL AUDIO_REG(AUDIO_RANGE_0, 0x08c)
>>>>>> +#define AUDIO_CLK_TDMOUT_A_CTRL AUDIO_REG(AUDIO_RANGE_0, 0x090)
>>>>>> +#define AUDIO_CLK_TDMOUT_B_CTRL AUDIO_REG(AUDIO_RANGE_0, 0x094)
>>>>>> +#define AUDIO_CLK_SPDIFIN_CTRL AUDIO_REG(AUDIO_RANGE_0, 0x09c)
>>>>>> +#define AUDIO_CLK_RESAMPLE_CTRL AUDIO_REG(AUDIO_RANGE_0, 0x0a4)
>>>>>> +#define AUDIO_CLK_LOCKER_CTRL AUDIO_REG(AUDIO_RANGE_0, 0x0a8)
>>>>>> +#define AUDIO_CLK_EQDRC_CTRL AUDIO_REG(AUDIO_RANGE_0, 0x0c0)
>>>>>> +
>>>>>> +#define AUDIO2_CLK_GATE_EN0 AUDIO_REG(AUDIO_RANGE_1, 0x00c)
>>>>>> +#define AUDIO2_MCLK_VAD_CTRL AUDIO_REG(AUDIO_RANGE_1, 0x040)
>>>>>> +#define AUDIO2_CLK_VAD_CTRL AUDIO_REG(AUDIO_RANGE_1, 0x044)
>>>>>> +#define AUDIO2_CLK_PDMIN_CTRL0 AUDIO_REG(AUDIO_RANGE_1, 0x058)
>>>>>> +#define AUDIO2_CLK_PDMIN_CTRL1 AUDIO_REG(AUDIO_RANGE_1, 0x05c)
>>>>>> +
>>>>>> +#include <dt-bindings/clock/amlogic,a1-audio-clkc.h>
>>>>>> +
>>>>>> +#endif /* __A1_AUDIO_H */
>>>>>
>>>>>
>>>
>>>
>
>
--
Best regards
Jan Dakinevich
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v5 05/27] iommu/arm-smmu-v3: Make CD programming use arm_smmu_write_entry()
From: Jason Gunthorpe @ 2024-03-26 18:30 UTC (permalink / raw)
To: Mostafa Saleh
Cc: iommu, Joerg Roedel, linux-arm-kernel, Robin Murphy, Will Deacon,
Eric Auger, Jean-Philippe Brucker, Moritz Fischer, Michael Shavit,
Nicolin Chen, patches, Shameerali Kolothum Thodi
In-Reply-To: <Zf7S109OKJlRVXiR@google.com>
On Sat, Mar 23, 2024 at 01:02:15PM +0000, Mostafa Saleh wrote:
> > +static void arm_smmu_get_cd_used(const __le64 *ent, __le64 *used_bits)
> > +{
> > + used_bits[0] = cpu_to_le64(CTXDESC_CD_0_V);
> > + if (!(ent[0] & cpu_to_le64(CTXDESC_CD_0_V)))
> > + return;
> > + memset(used_bits, 0xFF, sizeof(struct arm_smmu_cd));
>
> This is a slightly different approach than what the driver does for STEs,
> where it explicitly sets the used bits. Is there a reason for that?
It is just more compact this way
> > + /* EPD0 means T0SZ/TG0/IR0/OR0/SH0/TTB0 are IGNORED */
> > + if (ent[0] & cpu_to_le64(CTXDESC_CD_0_TCR_EPD0)) {
> > + used_bits[0] &= ~cpu_to_le64(
> > + CTXDESC_CD_0_TCR_T0SZ | CTXDESC_CD_0_TCR_TG0 |
> > + CTXDESC_CD_0_TCR_IRGN0 | CTXDESC_CD_0_TCR_ORGN0 |
> > + CTXDESC_CD_0_TCR_SH0);
> > + used_bits[1] &= ~cpu_to_le64(CTXDESC_CD_1_TTB0_MASK);
> > + }
> > +}
>
> We should add a comment about EPD1 maybe?
Driver doesn't use TTB1?
Jason
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox