2018-09-10 12:01:07

by Lubomir Rintel

[permalink] [raw]
Subject: [PATCH 0/5] Make SPI work on DT MMP2

This makes SPI work on an OPLC 1.75 that is a DT MMP2 platform.




2018-09-10 12:01:39

by Lubomir Rintel

[permalink] [raw]
Subject: [PATCH 2/5] PCI: provide pci_match_id() with CONFIG_PCI=n

This spares drivers from #ifdef-ing on CONFIG_PCI if the driver can be
optionally built on machines without PCI bus.

Consistent with acpi_driver_match_device() and similar.

Signed-off-by: Lubomir Rintel <[email protected]>
---
include/linux/pci.h | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/include/linux/pci.h b/include/linux/pci.h
index e72ca8dd6241..d2f14eb23ea4 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1702,6 +1702,10 @@ static inline int pci_irqd_intx_xlate(struct irq_domain *d,
unsigned long *out_hwirq,
unsigned int *out_type)
{ return -EINVAL; }
+
+static inline const struct pci_device_id *pci_match_id(const struct pci_device_id *ids,
+ struct pci_dev *dev)
+{ return NULL; }
#endif /* CONFIG_PCI */

/* Include architecture-dependent settings and functions */
--
2.17.1


2018-09-10 12:01:43

by Lubomir Rintel

[permalink] [raw]
Subject: [PATCH 4/5] spi: pxa2xx: add devicetree support

The MMP2 platform, that uses device tree, has this controller. Let's add
devicetree alongside platform & PCI.

Signed-off-by: Lubomir Rintel <[email protected]>
---
drivers/spi/spi-pxa2xx.c | 73 +++++++++++++++++++++++---------------
include/linux/pxa2xx_ssp.h | 1 +
2 files changed, 45 insertions(+), 29 deletions(-)

diff --git a/drivers/spi/spi-pxa2xx.c b/drivers/spi/spi-pxa2xx.c
index f674541675bb..58554c765a87 100644
--- a/drivers/spi/spi-pxa2xx.c
+++ b/drivers/spi/spi-pxa2xx.c
@@ -33,6 +33,7 @@
#include <linux/clk.h>
#include <linux/pm_runtime.h>
#include <linux/acpi.h>
+#include <linux/of_device.h>

#include "spi-pxa2xx.h"

@@ -1333,9 +1334,6 @@ static void cleanup(struct spi_device *spi)
kfree(chip);
}

-#ifdef CONFIG_PCI
-#ifdef CONFIG_ACPI
-
static const struct acpi_device_id pxa2xx_spi_acpi_match[] = {
{ "INT33C0", LPSS_LPT_SSP },
{ "INT33C1", LPSS_LPT_SSP },
@@ -1347,23 +1345,6 @@ static const struct acpi_device_id pxa2xx_spi_acpi_match[] = {
};
MODULE_DEVICE_TABLE(acpi, pxa2xx_spi_acpi_match);

-static int pxa2xx_spi_get_port_id(struct acpi_device *adev)
-{
- unsigned int devid;
- int port_id = -1;
-
- if (adev && adev->pnp.unique_id &&
- !kstrtouint(adev->pnp.unique_id, 0, &devid))
- port_id = devid;
- return port_id;
-}
-#else /* !CONFIG_ACPI */
-static int pxa2xx_spi_get_port_id(struct acpi_device *adev)
-{
- return -1;
-}
-#endif
-
/*
* PCI IDs of compound devices that integrate both host controller and private
* integrated DMA engine. Please note these are not used in module
@@ -1410,6 +1391,37 @@ static const struct pci_device_id pxa2xx_spi_pci_compound_match[] = {
{ },
};

+static const struct of_device_id pxa2xx_spi_of_match[] = {
+ { .compatible = "marvell,mmp2-ssp", .data = (void *)MMP2_SSP },
+ {},
+};
+MODULE_DEVICE_TABLE(of, pxa2xx_spi_of_match);
+
+#ifdef CONFIG_ACPI
+
+static int pxa2xx_spi_get_port_id(struct acpi_device *adev)
+{
+ unsigned int devid;
+ int port_id = -1;
+
+ if (adev && adev->pnp.unique_id &&
+ !kstrtouint(adev->pnp.unique_id, 0, &devid))
+ port_id = devid;
+ return port_id;
+}
+
+#else /* !CONFIG_ACPI */
+
+static int pxa2xx_spi_get_port_id(struct acpi_device *adev)
+{
+ return -1;
+}
+
+#endif /* CONFIG_ACPI */
+
+
+#ifdef CONFIG_PCI
+
static bool pxa2xx_spi_idma_filter(struct dma_chan *chan, void *param)
{
struct device *dev = param;
@@ -1420,6 +1432,8 @@ static bool pxa2xx_spi_idma_filter(struct dma_chan *chan, void *param)
return true;
}

+#endif /* CONFIG_PCI */
+
static struct pxa2xx_spi_master *
pxa2xx_spi_init_pdata(struct platform_device *pdev)
{
@@ -1429,11 +1443,15 @@ pxa2xx_spi_init_pdata(struct platform_device *pdev)
struct resource *res;
const struct acpi_device_id *adev_id = NULL;
const struct pci_device_id *pcidev_id = NULL;
+ const struct of_device_id *of_id = NULL;
enum pxa_ssp_type type;

adev = ACPI_COMPANION(&pdev->dev);

- if (dev_is_pci(pdev->dev.parent))
+ if (pdev->dev.of_node)
+ of_id = of_match_device(pdev->dev.driver->of_match_table,
+ &pdev->dev);
+ else if (dev_is_pci(pdev->dev.parent))
pcidev_id = pci_match_id(pxa2xx_spi_pci_compound_match,
to_pci_dev(pdev->dev.parent));
else if (adev)
@@ -1446,6 +1464,8 @@ pxa2xx_spi_init_pdata(struct platform_device *pdev)
type = (enum pxa_ssp_type)adev_id->driver_data;
else if (pcidev_id)
type = (enum pxa_ssp_type)pcidev_id->driver_data;
+ else if (of_id)
+ type = (enum pxa_ssp_type)of_id->data;
else
return NULL;

@@ -1464,11 +1484,13 @@ pxa2xx_spi_init_pdata(struct platform_device *pdev)
if (IS_ERR(ssp->mmio_base))
return NULL;

+#ifdef CONFIG_PCI
if (pcidev_id) {
pdata->tx_param = pdev->dev.parent;
pdata->rx_param = pdev->dev.parent;
pdata->dma_filter = pxa2xx_spi_idma_filter;
}
+#endif

ssp->clk = devm_clk_get(&pdev->dev, NULL);
ssp->irq = platform_get_irq(pdev, 0);
@@ -1482,14 +1504,6 @@ pxa2xx_spi_init_pdata(struct platform_device *pdev)
return pdata;
}

-#else /* !CONFIG_PCI */
-static inline struct pxa2xx_spi_master *
-pxa2xx_spi_init_pdata(struct platform_device *pdev)
-{
- return NULL;
-}
-#endif
-
static int pxa2xx_spi_fw_translate_cs(struct spi_controller *master,
unsigned int cs)
{
@@ -1848,6 +1862,7 @@ static struct platform_driver driver = {
.name = "pxa2xx-spi",
.pm = &pxa2xx_spi_pm_ops,
.acpi_match_table = ACPI_PTR(pxa2xx_spi_acpi_match),
+ .of_match_table = of_match_ptr(pxa2xx_spi_of_match),
},
.probe = pxa2xx_spi_probe,
.remove = pxa2xx_spi_remove,
diff --git a/include/linux/pxa2xx_ssp.h b/include/linux/pxa2xx_ssp.h
index 262e1f318836..979087e021f3 100644
--- a/include/linux/pxa2xx_ssp.h
+++ b/include/linux/pxa2xx_ssp.h
@@ -196,6 +196,7 @@ enum pxa_ssp_type {
PXA27x_SSP,
PXA3xx_SSP,
PXA168_SSP,
+ MMP2_SSP,
PXA910_SSP,
CE4100_SSP,
QUARK_X1000_SSP,
--
2.17.1


2018-09-10 12:01:41

by Lubomir Rintel

[permalink] [raw]
Subject: [PATCH 5/5] DT: marvell,mmp2: add SSP1 and SSP3

There seem to be SSP2, SSP4 and perhaps SSP5 too, but Marvel keeps their
base addresses secret.

The SSP1 and SSP3 addresses were taken from OLPC 1.75, OpenFirmware and
kernel respectively.

Signed-off-by: Lubomir Rintel <[email protected]>
---
arch/arm/boot/dts/mmp2.dtsi | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)

diff --git a/arch/arm/boot/dts/mmp2.dtsi b/arch/arm/boot/dts/mmp2.dtsi
index 766bbb8495b6..ba55d6483ed0 100644
--- a/arch/arm/boot/dts/mmp2.dtsi
+++ b/arch/arm/boot/dts/mmp2.dtsi
@@ -18,6 +18,8 @@
serial3 = &uart4;
i2c0 = &twsi1;
i2c1 = &twsi2;
+ spi0 = &ssp1;
+ spi3 = &ssp3;
};

soc {
@@ -239,6 +241,22 @@
resets = <&soc_clocks MMP2_CLK_RTC>;
status = "disabled";
};
+
+ ssp1: ssp@d4035000 {
+ compatible = "marvell,mmp2-ssp";
+ reg = <0xd4035000 0x1000>;
+ clocks = <&soc_clocks MMP2_CLK_SSP0>;
+ interrupts = <0>;
+ status = "disabled";
+ };
+
+ ssp3: ssp@d4037000 {
+ compatible = "marvell,mmp2-ssp";
+ reg = <0xd4037000 0x1000>;
+ clocks = <&soc_clocks MMP2_CLK_SSP2>;
+ interrupts = <20>;
+ status = "disabled";
+ };
};

soc_clocks: clocks{
--
2.17.1


2018-09-10 12:01:53

by Lubomir Rintel

[permalink] [raw]
Subject: [PATCH 3/5] spi: pxa2xx: use an enum for type

That seems to be the correct type.

Signed-off-by: Lubomir Rintel <[email protected]>
---
drivers/spi/spi-pxa2xx.c | 6 +++---
include/linux/pxa2xx_ssp.h | 2 +-
2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/spi-pxa2xx.c b/drivers/spi/spi-pxa2xx.c
index 14f4ea59caff..f674541675bb 100644
--- a/drivers/spi/spi-pxa2xx.c
+++ b/drivers/spi/spi-pxa2xx.c
@@ -1429,7 +1429,7 @@ pxa2xx_spi_init_pdata(struct platform_device *pdev)
struct resource *res;
const struct acpi_device_id *adev_id = NULL;
const struct pci_device_id *pcidev_id = NULL;
- int type;
+ enum pxa_ssp_type type;

adev = ACPI_COMPANION(&pdev->dev);

@@ -1443,9 +1443,9 @@ pxa2xx_spi_init_pdata(struct platform_device *pdev)
return NULL;

if (adev_id)
- type = (int)adev_id->driver_data;
+ type = (enum pxa_ssp_type)adev_id->driver_data;
else if (pcidev_id)
- type = (int)pcidev_id->driver_data;
+ type = (enum pxa_ssp_type)pcidev_id->driver_data;
else
return NULL;

diff --git a/include/linux/pxa2xx_ssp.h b/include/linux/pxa2xx_ssp.h
index 13b4244d44c1..262e1f318836 100644
--- a/include/linux/pxa2xx_ssp.h
+++ b/include/linux/pxa2xx_ssp.h
@@ -217,7 +217,7 @@ struct ssp_device {

const char *label;
int port_id;
- int type;
+ enum pxa_ssp_type type;
int use_count;
int irq;

--
2.17.1


2018-09-10 12:02:20

by Lubomir Rintel

[permalink] [raw]
Subject: [PATCH 1/5] dt-bindings: spi/spi-pxa2xx: add PXA2xx SSP SPI Controller

This is the SPI controller found on Marvel MMP2 and perhaps more
platforms.

Signed-off-by: Lubomir Rintel <[email protected]>
---
.../devicetree/bindings/spi/spi-pxa2xx.txt | 24 +++++++++++++++++++
1 file changed, 24 insertions(+)
create mode 100644 Documentation/devicetree/bindings/spi/spi-pxa2xx.txt

diff --git a/Documentation/devicetree/bindings/spi/spi-pxa2xx.txt b/Documentation/devicetree/bindings/spi/spi-pxa2xx.txt
new file mode 100644
index 000000000000..75be87cf9909
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/spi-pxa2xx.txt
@@ -0,0 +1,24 @@
+PXA2xx SSP SPI Controller
+
+Required properties:
+- compatible: Must be "marvell,mmp2-ssp".
+- reg: Offset and length of the device's register set.
+- interrupts: Should be the interrupt number.
+- clocks: Should contain a single entry describing the clock input.
+- #address-cells: Number of cells required to define a chip select address.
+- #size-cells: Should be zero.
+
+Optional properties:
+- cs-gpios: list of GPIO chip selects. See the SPI bus bindings,
+ Documentation/devicetree/bindings/spi/spi-bus.txt
+
+Child nodes represent devices on the SPI bus
+ See ../spi/spi-bus.txt
+
+Example:
+ ssp1: ssp@d4035000 {
+ compatible = "marvell,mmp2-ssp";
+ reg = <0xd4035000 0x1000>;
+ clocks = <&soc_clocks MMP2_CLK_SSP0>;
+ interrupts = <0>;
+ };
--
2.17.1


2018-09-12 17:19:37

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH 2/5] PCI: provide pci_match_id() with CONFIG_PCI=n

Please capitalize the subject as:

PCI: Provide pci_match_id() with CONFIG_PCI=n

On Mon, Sep 10, 2018 at 01:59:32PM +0200, Lubomir Rintel wrote:
> This spares drivers from #ifdef-ing on CONFIG_PCI if the driver can be
> optionally built on machines without PCI bus.
>
> Consistent with acpi_driver_match_device() and similar.
>
> Signed-off-by: Lubomir Rintel <[email protected]>

Acked-by: Bjorn Helgaas <[email protected]>

Thanks!

> ---
> include/linux/pci.h | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index e72ca8dd6241..d2f14eb23ea4 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1702,6 +1702,10 @@ static inline int pci_irqd_intx_xlate(struct irq_domain *d,
> unsigned long *out_hwirq,
> unsigned int *out_type)
> { return -EINVAL; }
> +
> +static inline const struct pci_device_id *pci_match_id(const struct pci_device_id *ids,
> + struct pci_dev *dev)
> +{ return NULL; }
> #endif /* CONFIG_PCI */
>
> /* Include architecture-dependent settings and functions */
> --
> 2.17.1
>

2018-09-21 20:34:52

by Robert Jarzmik

[permalink] [raw]
Subject: Re: [PATCH 3/5] spi: pxa2xx: use an enum for type

Lubomir Rintel <[email protected]> writes:

> That seems to be the correct type.
Okay, but what happens here when adev_id->driver_data is a value out of enum
range ? Does the following assignment make sense ?
> + type = (enum pxa_ssp_type)adev_id->driver_data;

As a side note, could you join for the next throw to the review :
- Jarkko Nikula <[email protected]>
- Mika Westerberg <[email protected]>

Even if they are Intel, I think they have worked a lot on this driver for Intel
platforms.

Cheers.

--
Robert

2018-10-09 16:44:35

by Lubomir Rintel

[permalink] [raw]
Subject: Re: [PATCH 3/5] spi: pxa2xx: use an enum for type

On Fri, 2018-09-21 at 22:34 +0200, Robert Jarzmik wrote:
> Lubomir Rintel <[email protected]> writes:
>
> > That seems to be the correct type.
> Okay, but what happens here when adev_id->driver_data is a value out
> of enum
> range ? Does the following assignment make sense ?
> > + type = (enum pxa_ssp_type)adev_id->driver_data;

No change in behavior. In standard C the enum type is compatible with
an integer type (char or larger), thus type would just hold a value
outside the rank of an enum.

But why would that happen? What we can get here is just the constants
from pxa2xx_spi_acpi_match[], all of them of enum pxa_ssp_type type.

> As a side note, could you join for the next throw to the review :
> - Jarkko Nikula <[email protected]>
> - Mika Westerberg <[email protected]>
>
> Even if they are Intel, I think they have worked a lot on this driver
> for Intel
> platforms.

Will do.

> Cheers.

Thanks,
Lubo


2018-10-10 17:28:51

by Lubomir Rintel

[permalink] [raw]
Subject: Re: [PATCH 3/5] spi: pxa2xx: use an enum for type

On Fri, 2018-09-21 at 22:34 +0200, Robert Jarzmik wrote:
> Lubomir Rintel <[email protected]> writes:
>
> > That seems to be the correct type.
> Okay, but what happens here when adev_id->driver_data is a value out
> of enum
> range ? Does the following assignment make sense ?
> > + type = (enum pxa_ssp_type)adev_id->driver_data;
>
> As a side note, could you join for the next throw to the review :
> - Jarkko Nikula <[email protected]>
> - Mika Westerberg <[email protected]>

Argh, of course I forgot. My apologies.

Jarkko & Mika, I'm wondering if you could take a look at the [1] thread
and tell me what you think. (I can forward it to you via e-mail too, if
you're not subscribed to lists it has been Cc'd to, if that would be
more convenient).

[1] [PATCH 0/11] spi: pxa2xx: add DT and slave mode support"
https://lore.kernel.org/lkml/[email protected]/T/#t

Thanks
Lubo

>
> Even if they are Intel, I think they have worked a lot on this driver
> for Intel
> platforms.
>
> Cheers.
>