This series is attempt to revive previous work to add support for SPI
controller which is used in newest Allwinner's SOCs R329/D1/R528/T113s
https://lore.kernel.org/lkml/BYAPR20MB2472E8B10BFEF75E7950BBC0BCF79@BYAPR20MB2472.namprd20.prod.outlook.com/
Previous discussion about D1/T113s support
https://lore.kernel.org/linux-arm-kernel/[email protected]/T/
v2:
- added DT bindings and node for D1/T113s
Icenowy Zheng (4):
dt-bindings: spi: sun6i: add DT bindings for Allwinner R329 SPI
spi: sun6i: change OF match data to a struct
spi: sun6i: add quirk for in-controller clock divider
spi: sun6i: add support for R329 SPI controllers
Maksim Kiselev (2):
dt-bindings: spi: sun6i: add DT bindings for Allwinner D1/R528/T113s
SPI
riscv: dts: allwinner: d1: Add SPI0 controller node
.../bindings/spi/allwinner,sun6i-a31-spi.yaml | 6 +
.../boot/dts/allwinner/sunxi-d1s-t113.dtsi | 21 ++++
drivers/spi/spi-sun6i.c | 112 +++++++++++-------
3 files changed, 99 insertions(+), 40 deletions(-)
--
2.39.2
From: Icenowy Zheng <[email protected]>
R329 has two SPI controllers. One of it is quite similar to previous
ones, but with internal clock divider removed; the other added MIPI DBI
Type-C offload based on the first one.
Add basical support for these controllers. As we're not going to
support the DBI functionality now, just implement the two kinds of
controllers as the same.
Signed-off-by: Icenowy Zheng <[email protected]>
---
drivers/spi/spi-sun6i.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/drivers/spi/spi-sun6i.c b/drivers/spi/spi-sun6i.c
index 82523011a3a5..fe287a45df9b 100644
--- a/drivers/spi/spi-sun6i.c
+++ b/drivers/spi/spi-sun6i.c
@@ -715,9 +715,21 @@ static const struct sun6i_spi_cfg sun8i_h3_spi_cfg = {
.has_clk_ctl = true,
};
+static const struct sun6i_spi_cfg sun50i_r329_spi_cfg = {
+ .fifo_depth = SUN8I_FIFO_DEPTH,
+};
+
static const struct of_device_id sun6i_spi_match[] = {
{ .compatible = "allwinner,sun6i-a31-spi", .data = &sun6i_a31_spi_cfg },
{ .compatible = "allwinner,sun8i-h3-spi", .data = &sun8i_h3_spi_cfg },
+ {
+ .compatible = "allwinner,sun50i-r329-spi",
+ .data = &sun50i_r329_spi_cfg
+ },
+ {
+ .compatible = "allwinner,sun50i-r329-spi-dbi",
+ .data = &sun50i_r329_spi_cfg
+ },
{}
};
MODULE_DEVICE_TABLE(of, sun6i_spi_match);
--
2.39.2
From: Icenowy Zheng <[email protected]>
As we're adding more properties to the OF match data, convert it to a
struct now.
Signed-off-by: Icenowy Zheng <[email protected]>
---
drivers/spi/spi-sun6i.c | 32 ++++++++++++++++++++++----------
1 file changed, 22 insertions(+), 10 deletions(-)
diff --git a/drivers/spi/spi-sun6i.c b/drivers/spi/spi-sun6i.c
index 7532c85a352c..01a01cd86db5 100644
--- a/drivers/spi/spi-sun6i.c
+++ b/drivers/spi/spi-sun6i.c
@@ -85,6 +85,10 @@
#define SUN6I_TXDATA_REG 0x200
#define SUN6I_RXDATA_REG 0x300
+struct sun6i_spi_cfg {
+ unsigned long fifo_depth;
+};
+
struct sun6i_spi {
struct spi_master *master;
void __iomem *base_addr;
@@ -99,7 +103,7 @@ struct sun6i_spi {
const u8 *tx_buf;
u8 *rx_buf;
int len;
- unsigned long fifo_depth;
+ const struct sun6i_spi_cfg *cfg;
};
static inline u32 sun6i_spi_read(struct sun6i_spi *sspi, u32 reg)
@@ -156,7 +160,7 @@ static inline void sun6i_spi_fill_fifo(struct sun6i_spi *sspi)
u8 byte;
/* See how much data we can fit */
- cnt = sspi->fifo_depth - sun6i_spi_get_tx_fifo_count(sspi);
+ cnt = sspi->cfg->fifo_depth - sun6i_spi_get_tx_fifo_count(sspi);
len = min((int)cnt, sspi->len);
@@ -289,14 +293,14 @@ static int sun6i_spi_transfer_one(struct spi_master *master,
* the hardcoded value used in old generation of Allwinner
* SPI controller. (See spi-sun4i.c)
*/
- trig_level = sspi->fifo_depth / 4 * 3;
+ trig_level = sspi->cfg->fifo_depth / 4 * 3;
} else {
/*
* Setup FIFO DMA request trigger level
* We choose 1/2 of the full fifo depth, that value will
* be used as DMA burst length.
*/
- trig_level = sspi->fifo_depth / 2;
+ trig_level = sspi->cfg->fifo_depth / 2;
if (tfr->tx_buf)
reg |= SUN6I_FIFO_CTL_TF_DRQ_EN;
@@ -410,9 +414,9 @@ static int sun6i_spi_transfer_one(struct spi_master *master,
reg = SUN6I_INT_CTL_TC;
if (!use_dma) {
- if (rx_len > sspi->fifo_depth)
+ if (rx_len > sspi->cfg->fifo_depth)
reg |= SUN6I_INT_CTL_RF_RDY;
- if (tx_len > sspi->fifo_depth)
+ if (tx_len > sspi->cfg->fifo_depth)
reg |= SUN6I_INT_CTL_TF_ERQ;
}
@@ -543,7 +547,7 @@ static bool sun6i_spi_can_dma(struct spi_master *master,
* the fifo length we can just fill the fifo and wait for a single
* irq, so don't bother setting up dma
*/
- return xfer->len > sspi->fifo_depth;
+ return xfer->len > sspi->cfg->fifo_depth;
}
static int sun6i_spi_probe(struct platform_device *pdev)
@@ -582,7 +586,7 @@ static int sun6i_spi_probe(struct platform_device *pdev)
}
sspi->master = master;
- sspi->fifo_depth = (unsigned long)of_device_get_match_data(&pdev->dev);
+ sspi->cfg = of_device_get_match_data(&pdev->dev);
master->max_speed_hz = 100 * 1000 * 1000;
master->min_speed_hz = 3 * 1000;
@@ -695,9 +699,17 @@ static void sun6i_spi_remove(struct platform_device *pdev)
dma_release_channel(master->dma_rx);
}
+static const struct sun6i_spi_cfg sun6i_a31_spi_cfg = {
+ .fifo_depth = SUN6I_FIFO_DEPTH,
+};
+
+static const struct sun6i_spi_cfg sun8i_h3_spi_cfg = {
+ .fifo_depth = SUN8I_FIFO_DEPTH,
+};
+
static const struct of_device_id sun6i_spi_match[] = {
- { .compatible = "allwinner,sun6i-a31-spi", .data = (void *)SUN6I_FIFO_DEPTH },
- { .compatible = "allwinner,sun8i-h3-spi", .data = (void *)SUN8I_FIFO_DEPTH },
+ { .compatible = "allwinner,sun6i-a31-spi", .data = &sun6i_a31_spi_cfg },
+ { .compatible = "allwinner,sun8i-h3-spi", .data = &sun8i_h3_spi_cfg },
{}
};
MODULE_DEVICE_TABLE(of, sun6i_spi_match);
--
2.39.2
Some boards form the MangoPi family (MQ\MQ-Dual\MQ-R) may have
an optional SPI flash that connects to the SPI0 controller.
This controller is the same for R329/D1/R528/T113s SoCs and
should be supported by the sun50i-r329-spi driver.
So let's add its DT node.
Signed-off-by: Maksim Kiselev <[email protected]>
---
.../boot/dts/allwinner/sunxi-d1s-t113.dtsi | 21 +++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi b/arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi
index 922e8e0e2c09..a52999240a8e 100644
--- a/arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi
+++ b/arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi
@@ -108,6 +108,12 @@ rmii_pe_pins: rmii-pe-pins {
function = "emac";
};
+ /omit-if-no-ref/
+ spi0_pins: spi0-pins {
+ pins = "PC2", "PC3", "PC4", "PC5";
+ function = "spi0";
+ };
+
/omit-if-no-ref/
uart1_pg6_pins: uart1-pg6-pins {
pins = "PG6", "PG7";
@@ -447,6 +453,21 @@ mmc2: mmc@4022000 {
#size-cells = <0>;
};
+ spi0: spi@4025000 {
+ compatible = "allwinner,sun20i-d1-spi",
+ "allwinner,sun50i-r329-spi";
+ reg = <0x04025000 0x300>;
+ interrupts = <SOC_PERIPHERAL_IRQ(15) IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&ccu CLK_BUS_SPI0>, <&ccu CLK_SPI0>;
+ clock-names = "ahb", "mod";
+ dmas = <&dma 22>, <&dma 22>;
+ dma-names = "rx", "tx";
+ resets = <&ccu RST_BUS_SPI0>;
+ status = "disabled";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
+
usb_otg: usb@4100000 {
compatible = "allwinner,sun20i-d1-musb",
"allwinner,sun8i-a33-musb";
--
2.39.2
Allwinner D1/R528/T113s SPI has the same as R329 controllers
Add compatible string for this controller
Signed-off-by: Maksim Kiselev <[email protected]>
---
.../devicetree/bindings/spi/allwinner,sun6i-a31-spi.yaml | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/Documentation/devicetree/bindings/spi/allwinner,sun6i-a31-spi.yaml b/Documentation/devicetree/bindings/spi/allwinner,sun6i-a31-spi.yaml
index 2c1b8da35339..164bd6af9299 100644
--- a/Documentation/devicetree/bindings/spi/allwinner,sun6i-a31-spi.yaml
+++ b/Documentation/devicetree/bindings/spi/allwinner,sun6i-a31-spi.yaml
@@ -30,6 +30,10 @@ properties:
- allwinner,sun50i-h616-spi
- allwinner,suniv-f1c100s-spi
- const: allwinner,sun8i-h3-spi
+ - items:
+ - enum:
+ - allwinner,sun20i-d1-spi
+ - const: allwinner,sun50i-r329-spi
reg:
maxItems: 1
--
2.39.2
From: Icenowy Zheng <[email protected]>
Previously SPI controllers in Allwinner SoCs has a clock divider inside.
However now the clock divider is removed and to set the transfer clock
rate it's only needed to set the SPI module clock to the target value.
Add a quirk for this kind of SPI controllers.
Signed-off-by: Icenowy Zheng <[email protected]>
---
drivers/spi/spi-sun6i.c | 68 +++++++++++++++++++++++------------------
1 file changed, 38 insertions(+), 30 deletions(-)
diff --git a/drivers/spi/spi-sun6i.c b/drivers/spi/spi-sun6i.c
index 01a01cd86db5..82523011a3a5 100644
--- a/drivers/spi/spi-sun6i.c
+++ b/drivers/spi/spi-sun6i.c
@@ -87,6 +87,7 @@
struct sun6i_spi_cfg {
unsigned long fifo_depth;
+ bool has_clk_ctl;
};
struct sun6i_spi {
@@ -260,7 +261,7 @@ static int sun6i_spi_transfer_one(struct spi_master *master,
struct spi_transfer *tfr)
{
struct sun6i_spi *sspi = spi_master_get_devdata(master);
- unsigned int mclk_rate, div, div_cdr1, div_cdr2, timeout;
+ unsigned int div, div_cdr1, div_cdr2, timeout;
unsigned int start, end, tx_time;
unsigned int trig_level;
unsigned int tx_len = 0, rx_len = 0;
@@ -350,39 +351,44 @@ static int sun6i_spi_transfer_one(struct spi_master *master,
sun6i_spi_write(sspi, SUN6I_TFR_CTL_REG, reg);
- /* Ensure that we have a parent clock fast enough */
- mclk_rate = clk_get_rate(sspi->mclk);
- if (mclk_rate < (2 * tfr->speed_hz)) {
- clk_set_rate(sspi->mclk, 2 * tfr->speed_hz);
- mclk_rate = clk_get_rate(sspi->mclk);
- }
+ if (sspi->cfg->has_clk_ctl) {
+ unsigned int mclk_rate = clk_get_rate(sspi->mclk);
+ /* Ensure that we have a parent clock fast enough */
+ if (mclk_rate < (2 * tfr->speed_hz)) {
+ clk_set_rate(sspi->mclk, 2 * tfr->speed_hz);
+ mclk_rate = clk_get_rate(sspi->mclk);
+ }
- /*
- * Setup clock divider.
- *
- * We have two choices there. Either we can use the clock
- * divide rate 1, which is calculated thanks to this formula:
- * SPI_CLK = MOD_CLK / (2 ^ cdr)
- * Or we can use CDR2, which is calculated with the formula:
- * SPI_CLK = MOD_CLK / (2 * (cdr + 1))
- * Wether we use the former or the latter is set through the
- * DRS bit.
- *
- * First try CDR2, and if we can't reach the expected
- * frequency, fall back to CDR1.
- */
- div_cdr1 = DIV_ROUND_UP(mclk_rate, tfr->speed_hz);
- div_cdr2 = DIV_ROUND_UP(div_cdr1, 2);
- if (div_cdr2 <= (SUN6I_CLK_CTL_CDR2_MASK + 1)) {
- reg = SUN6I_CLK_CTL_CDR2(div_cdr2 - 1) | SUN6I_CLK_CTL_DRS;
- tfr->effective_speed_hz = mclk_rate / (2 * div_cdr2);
+ /*
+ * Setup clock divider.
+ *
+ * We have two choices there. Either we can use the clock
+ * divide rate 1, which is calculated thanks to this formula:
+ * SPI_CLK = MOD_CLK / (2 ^ cdr)
+ * Or we can use CDR2, which is calculated with the formula:
+ * SPI_CLK = MOD_CLK / (2 * (cdr + 1))
+ * Wether we use the former or the latter is set through the
+ * DRS bit.
+ *
+ * First try CDR2, and if we can't reach the expected
+ * frequency, fall back to CDR1.
+ */
+ div_cdr1 = DIV_ROUND_UP(mclk_rate, tfr->speed_hz);
+ div_cdr2 = DIV_ROUND_UP(div_cdr1, 2);
+ if (div_cdr2 <= (SUN6I_CLK_CTL_CDR2_MASK + 1)) {
+ reg = SUN6I_CLK_CTL_CDR2(div_cdr2 - 1) | SUN6I_CLK_CTL_DRS;
+ tfr->effective_speed_hz = mclk_rate / (2 * div_cdr2);
+ } else {
+ div = min(SUN6I_CLK_CTL_CDR1_MASK, order_base_2(div_cdr1));
+ reg = SUN6I_CLK_CTL_CDR1(div);
+ tfr->effective_speed_hz = mclk_rate / (1 << div);
+ }
+
+ sun6i_spi_write(sspi, SUN6I_CLK_CTL_REG, reg);
} else {
- div = min(SUN6I_CLK_CTL_CDR1_MASK, order_base_2(div_cdr1));
- reg = SUN6I_CLK_CTL_CDR1(div);
- tfr->effective_speed_hz = mclk_rate / (1 << div);
+ clk_set_rate(sspi->mclk, tfr->speed_hz);
}
- sun6i_spi_write(sspi, SUN6I_CLK_CTL_REG, reg);
/* Finally enable the bus - doing so before might raise SCK to HIGH */
reg = sun6i_spi_read(sspi, SUN6I_GBL_CTL_REG);
reg |= SUN6I_GBL_CTL_BUS_ENABLE;
@@ -701,10 +707,12 @@ static void sun6i_spi_remove(struct platform_device *pdev)
static const struct sun6i_spi_cfg sun6i_a31_spi_cfg = {
.fifo_depth = SUN6I_FIFO_DEPTH,
+ .has_clk_ctl = true,
};
static const struct sun6i_spi_cfg sun8i_h3_spi_cfg = {
.fifo_depth = SUN8I_FIFO_DEPTH,
+ .has_clk_ctl = true,
};
static const struct of_device_id sun6i_spi_match[] = {
--
2.39.2
From: Icenowy Zheng <[email protected]>
Allwinner R329 SPI has two controllers, and the second one has helper
functions for MIPI-DBI Type C.
Add compatible strings for these controllers
Signed-off-by: Icenowy Zheng <[email protected]>
---
.../devicetree/bindings/spi/allwinner,sun6i-a31-spi.yaml | 2 ++
1 file changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/spi/allwinner,sun6i-a31-spi.yaml b/Documentation/devicetree/bindings/spi/allwinner,sun6i-a31-spi.yaml
index de36c6a34a0f..2c1b8da35339 100644
--- a/Documentation/devicetree/bindings/spi/allwinner,sun6i-a31-spi.yaml
+++ b/Documentation/devicetree/bindings/spi/allwinner,sun6i-a31-spi.yaml
@@ -21,6 +21,8 @@ properties:
oneOf:
- const: allwinner,sun6i-a31-spi
- const: allwinner,sun8i-h3-spi
+ - const: allwinner,sun50i-r329-spi
+ - const: allwinner,sun50i-r329-spi-dbi
- items:
- enum:
- allwinner,sun8i-r40-spi
--
2.39.2
Hey Maksim,
On Sat, May 06, 2023 at 10:30:09AM +0300, Maksim Kiselev wrote:
> From: Icenowy Zheng <[email protected]>
>
> Allwinner R329 SPI has two controllers, and the second one has helper
> functions for MIPI-DBI Type C.
>
> Add compatible strings for these controllers
>
> Signed-off-by: Icenowy Zheng <[email protected]>
> ---
> .../devicetree/bindings/spi/allwinner,sun6i-a31-spi.yaml | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/spi/allwinner,sun6i-a31-spi.yaml b/Documentation/devicetree/bindings/spi/allwinner,sun6i-a31-spi.yaml
> index de36c6a34a0f..2c1b8da35339 100644
> --- a/Documentation/devicetree/bindings/spi/allwinner,sun6i-a31-spi.yaml
> +++ b/Documentation/devicetree/bindings/spi/allwinner,sun6i-a31-spi.yaml
> @@ -21,6 +21,8 @@ properties:
> oneOf:
> - const: allwinner,sun6i-a31-spi
> - const: allwinner,sun8i-h3-spi
> + - const: allwinner,sun50i-r329-spi
> + - const: allwinner,sun50i-r329-spi-dbi
From the driver patch:
"Add basical support for these controllers. As we're not going to
support the DBI functionality now, just implement the two kinds of
controllers as the same."
Should this not be set up as a fallback compatible, per Samuel's
suggestion here:
https://lore.kernel.org/lkml/[email protected]/
Thanks,
Conor.
> - items:
> - enum:
> - allwinner,sun8i-r40-spi
> --
> 2.39.2
>
On Sat, May 06, 2023 at 10:30:14AM +0300, Maksim Kiselev wrote:
> Some boards form the MangoPi family (MQ\MQ-Dual\MQ-R) may have
> an optional SPI flash that connects to the SPI0 controller.
>
> This controller is the same for R329/D1/R528/T113s SoCs and
> should be supported by the sun50i-r329-spi driver.
>
> So let's add its DT node.
>
> Signed-off-by: Maksim Kiselev <[email protected]>
Acked-by: Conor Dooley <[email protected]>
Thanks,
Conor.
On Sat, May 06, 2023 at 10:30:13AM +0300, Maksim Kiselev wrote:
> Allwinner D1/R528/T113s SPI has the same as R329 controllers
>
> Add compatible string for this controller
>
> Signed-off-by: Maksim Kiselev <[email protected]>
> ---
> .../devicetree/bindings/spi/allwinner,sun6i-a31-spi.yaml | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/spi/allwinner,sun6i-a31-spi.yaml b/Documentation/devicetree/bindings/spi/allwinner,sun6i-a31-spi.yaml
> index 2c1b8da35339..164bd6af9299 100644
> --- a/Documentation/devicetree/bindings/spi/allwinner,sun6i-a31-spi.yaml
> +++ b/Documentation/devicetree/bindings/spi/allwinner,sun6i-a31-spi.yaml
> @@ -30,6 +30,10 @@ properties:
> - allwinner,sun50i-h616-spi
> - allwinner,suniv-f1c100s-spi
> - const: allwinner,sun8i-h3-spi
> + - items:
> + - enum:
> + - allwinner,sun20i-d1-spi
Yah, this one is more like it. The "allwinner,sun50i-r329-spi-dbi" one
should be done in the way too.
Cheers,
Conor.
> + - const: allwinner,sun50i-r329-spi
>
> reg:
> maxItems: 1
> --
> 2.39.2
>
On 06/05/2023 09:30, Maksim Kiselev wrote:
> From: Icenowy Zheng <[email protected]>
>
> Allwinner R329 SPI has two controllers, and the second one has helper
> functions for MIPI-DBI Type C.
I wonder what is the difference of DBI compatible. You refer to "helper
functions", which sounds like driver... do you mean some parts of SPI
controller?
>
> Add compatible strings for these controllers
>
> Signed-off-by: Icenowy Zheng <[email protected]>
> ---
> .../devicetree/bindings/spi/allwinner,sun6i-a31-spi.yaml | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/spi/allwinner,sun6i-a31-spi.yaml b/Documentation/devicetree/bindings/spi/allwinner,sun6i-a31-spi.yaml
> index de36c6a34a0f..2c1b8da35339 100644
> --- a/Documentation/devicetree/bindings/spi/allwinner,sun6i-a31-spi.yaml
> +++ b/Documentation/devicetree/bindings/spi/allwinner,sun6i-a31-spi.yaml
> @@ -21,6 +21,8 @@ properties:
> oneOf:
> - const: allwinner,sun6i-a31-spi
> - const: allwinner,sun8i-h3-spi
> + - const: allwinner,sun50i-r329-spi
> + - const: allwinner,sun50i-r329-spi-dbi
As Conor pointed out, nothing improved here.
Best regards,
Krzysztof
> Should this not be set up as a fallback compatible, per Samuel's
> suggestion here:
Ok, I'll do it in the next version.
> I wonder what is the difference of DBI compatible. You refer to "helper
> functions", which sounds like driver... do you mean some parts of SPI
> controller?
According to the D1 datasheet the SPI_DBI controller uses the same
registers layout as the regular SPI0 controller.
But also it has an additional DBI mode functionality. Support for this
mode is not yet implemented.
So there is no difference between 'sun50i-r329-spi' and
'sun50i-r329-spi-dbi' controllers types in the SPI driver.
Maybe we should drop 'sun50i-r329-spi-dbi' compatible struct from here
https://lore.kernel.org/lkml/[email protected]/
for a while the DBI mode functionality will not be implemented?
On Sat, 6 May 2023 12:53:07 +0200
Krzysztof Kozlowski <[email protected]> wrote:
Hi Maksim,
> On 06/05/2023 09:30, Maksim Kiselev wrote:
> > From: Icenowy Zheng <[email protected]>
> >
> > Allwinner R329 SPI has two controllers, and the second one has helper
> > functions for MIPI-DBI Type C.
>
> I wonder what is the difference of DBI compatible. You refer to "helper
> functions", which sounds like driver... do you mean some parts of SPI
> controller?
So I checked the manuals, all of the D1, T113-s and R329 are the same
in this respect:
- There are two SPI controllers, the "first" one is what this series
fully supports.
- The second SPI controller has some additional registers and two
extra bits in the control register to drive the DBI extension, but is
otherwise compatible to the first one:
So I'd suggest to introduce the following compatible string
combinations to the binding *now*. We don't support the DBI bits (yet),
but this would be the correct hardware description:
For the R329:
spi0: spi@4025000 {
compatible = "allwinner,sun50i-r329-spi";
....
spi1: spi@4026000 {
compatible = "allwinner,sun50i-r329-spi-dbi",
"allwinner,sun50i-r329-spi";
For the D1/T113s:
spi0: spi@4025000 {
compatible = "allwinner,sun20i-d1-spi",
"allwinner,sun50i-r329-spi";
....
spi1: spi@4026000 {
compatible = "allwinner,sun20i-d1-spi-dbi",
"allwinner,sun50i-r329-spi-dbi",
"allwinner,sun50i-r329-spi";
I leave that as an exercise to the reader to convert that into the
minimal set of DT schema lines ;-)
I would suggest to add both the D1/T113s and the R329 bindings in this
one patch, to reduce the churn. I guess if you do this, you could even
drop Icenowy's authorship on this patch, since it has not much to do
with the original version anymore.
Cheers,
Andre
> > Add compatible strings for these controllers
> >
> > Signed-off-by: Icenowy Zheng <[email protected]>
>
> > ---
> > .../devicetree/bindings/spi/allwinner,sun6i-a31-spi.yaml | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/spi/allwinner,sun6i-a31-spi.yaml b/Documentation/devicetree/bindings/spi/allwinner,sun6i-a31-spi.yaml
> > index de36c6a34a0f..2c1b8da35339 100644
> > --- a/Documentation/devicetree/bindings/spi/allwinner,sun6i-a31-spi.yaml
> > +++ b/Documentation/devicetree/bindings/spi/allwinner,sun6i-a31-spi.yaml
> > @@ -21,6 +21,8 @@ properties:
> > oneOf:
> > - const: allwinner,sun6i-a31-spi
> > - const: allwinner,sun8i-h3-spi
> > + - const:
> > + - const: allwinner,sun50i-r329-spi-dbi
>
> As Conor pointed out, nothing improved here.
>
> Best regards,
> Krzysztof
>
On Sat, 6 May 2023 10:30:12 +0300
Maksim Kiselev <[email protected]> wrote:
> From: Icenowy Zheng <[email protected]>
>
> R329 has two SPI controllers. One of it is quite similar to previous
> ones, but with internal clock divider removed; the other added MIPI DBI
> Type-C offload based on the first one.
>
> Add basical support for these controllers. As we're not going to
> support the DBI functionality now, just implement the two kinds of
> controllers as the same.
>
> Signed-off-by: Icenowy Zheng <[email protected]>
> ---
> drivers/spi/spi-sun6i.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/drivers/spi/spi-sun6i.c b/drivers/spi/spi-sun6i.c
> index 82523011a3a5..fe287a45df9b 100644
> --- a/drivers/spi/spi-sun6i.c
> +++ b/drivers/spi/spi-sun6i.c
> @@ -715,9 +715,21 @@ static const struct sun6i_spi_cfg sun8i_h3_spi_cfg = {
> .has_clk_ctl = true,
> };
>
> +static const struct sun6i_spi_cfg sun50i_r329_spi_cfg = {
> + .fifo_depth = SUN8I_FIFO_DEPTH,
> +};
> +
> static const struct of_device_id sun6i_spi_match[] = {
> { .compatible = "allwinner,sun6i-a31-spi", .data = &sun6i_a31_spi_cfg },
> { .compatible = "allwinner,sun8i-h3-spi", .data = &sun8i_h3_spi_cfg },
> + {
> + .compatible = "allwinner,sun50i-r329-spi",
> + .data = &sun50i_r329_spi_cfg
> + },
> + {
> + .compatible = "allwinner,sun50i-r329-spi-dbi",
> + .data = &sun50i_r329_spi_cfg
> + },
As mentioned by others, we would not need to mention this compatible
string in the driver, the fallback string in the DT should take care
of the detection.
Rest looks fine.
Cheers,
Andre
> {}
> };
> MODULE_DEVICE_TABLE(of, sun6i_spi_match);
On Sat, 6 May 2023 10:30:11 +0300
Maksim Kiselev <[email protected]> wrote:
> From: Icenowy Zheng <[email protected]>
>
> Previously SPI controllers in Allwinner SoCs has a clock divider inside.
> However now the clock divider is removed and to set the transfer clock
> rate it's only needed to set the SPI module clock to the target value.
>
> Add a quirk for this kind of SPI controllers.
>
> Signed-off-by: Icenowy Zheng <[email protected]>
> ---
> drivers/spi/spi-sun6i.c | 68 +++++++++++++++++++++++------------------
> 1 file changed, 38 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/spi/spi-sun6i.c b/drivers/spi/spi-sun6i.c
> index 01a01cd86db5..82523011a3a5 100644
> --- a/drivers/spi/spi-sun6i.c
> +++ b/drivers/spi/spi-sun6i.c
> @@ -87,6 +87,7 @@
>
> struct sun6i_spi_cfg {
> unsigned long fifo_depth;
> + bool has_clk_ctl;
> };
>
> struct sun6i_spi {
> @@ -260,7 +261,7 @@ static int sun6i_spi_transfer_one(struct spi_master *master,
> struct spi_transfer *tfr)
> {
> struct sun6i_spi *sspi = spi_master_get_devdata(master);
> - unsigned int mclk_rate, div, div_cdr1, div_cdr2, timeout;
> + unsigned int div, div_cdr1, div_cdr2, timeout;
> unsigned int start, end, tx_time;
> unsigned int trig_level;
> unsigned int tx_len = 0, rx_len = 0;
> @@ -350,39 +351,44 @@ static int sun6i_spi_transfer_one(struct spi_master *master,
>
> sun6i_spi_write(sspi, SUN6I_TFR_CTL_REG, reg);
>
> - /* Ensure that we have a parent clock fast enough */
> - mclk_rate = clk_get_rate(sspi->mclk);
> - if (mclk_rate < (2 * tfr->speed_hz)) {
> - clk_set_rate(sspi->mclk, 2 * tfr->speed_hz);
> - mclk_rate = clk_get_rate(sspi->mclk);
> - }
> + if (sspi->cfg->has_clk_ctl) {
> + unsigned int mclk_rate = clk_get_rate(sspi->mclk);
New line here please, to separate variable declaration from code.
> + /* Ensure that we have a parent clock fast enough */
> + if (mclk_rate < (2 * tfr->speed_hz)) {
> + clk_set_rate(sspi->mclk, 2 * tfr->speed_hz);
> + mclk_rate = clk_get_rate(sspi->mclk);
> + }
>
> - /*
> - * Setup clock divider.
> - *
> - * We have two choices there. Either we can use the clock
> - * divide rate 1, which is calculated thanks to this formula:
> - * SPI_CLK = MOD_CLK / (2 ^ cdr)
> - * Or we can use CDR2, which is calculated with the formula:
> - * SPI_CLK = MOD_CLK / (2 * (cdr + 1))
> - * Wether we use the former or the latter is set through the
> - * DRS bit.
> - *
> - * First try CDR2, and if we can't reach the expected
> - * frequency, fall back to CDR1.
> - */
> - div_cdr1 = DIV_ROUND_UP(mclk_rate, tfr->speed_hz);
> - div_cdr2 = DIV_ROUND_UP(div_cdr1, 2);
> - if (div_cdr2 <= (SUN6I_CLK_CTL_CDR2_MASK + 1)) {
> - reg = SUN6I_CLK_CTL_CDR2(div_cdr2 - 1) | SUN6I_CLK_CTL_DRS;
> - tfr->effective_speed_hz = mclk_rate / (2 * div_cdr2);
> + /*
> + * Setup clock divider.
> + *
> + * We have two choices there. Either we can use the clock
> + * divide rate 1, which is calculated thanks to this formula:
> + * SPI_CLK = MOD_CLK / (2 ^ cdr)
> + * Or we can use CDR2, which is calculated with the formula:
> + * SPI_CLK = MOD_CLK / (2 * (cdr + 1))
> + * Wether we use the former or the latter is set through the
> + * DRS bit.
> + *
> + * First try CDR2, and if we can't reach the expected
> + * frequency, fall back to CDR1.
> + */
> + div_cdr1 = DIV_ROUND_UP(mclk_rate, tfr->speed_hz);
> + div_cdr2 = DIV_ROUND_UP(div_cdr1, 2);
> + if (div_cdr2 <= (SUN6I_CLK_CTL_CDR2_MASK + 1)) {
> + reg = SUN6I_CLK_CTL_CDR2(div_cdr2 - 1) | SUN6I_CLK_CTL_DRS;
> + tfr->effective_speed_hz = mclk_rate / (2 * div_cdr2);
> + } else {
> + div = min(SUN6I_CLK_CTL_CDR1_MASK, order_base_2(div_cdr1));
> + reg = SUN6I_CLK_CTL_CDR1(div);
> + tfr->effective_speed_hz = mclk_rate / (1 << div);
> + }
> +
> + sun6i_spi_write(sspi, SUN6I_CLK_CTL_REG, reg);
> } else {
> - div = min(SUN6I_CLK_CTL_CDR1_MASK, order_base_2(div_cdr1));
> - reg = SUN6I_CLK_CTL_CDR1(div);
> - tfr->effective_speed_hz = mclk_rate / (1 << div);
> + clk_set_rate(sspi->mclk, tfr->speed_hz);
Don't we need to set tfr->effective_speed_hz to the actually programmed
clock rate here?
The rest looks fine, it's really mostly that old block indented.
Cheers,
Andre
> }
>
> - sun6i_spi_write(sspi, SUN6I_CLK_CTL_REG, reg);
> /* Finally enable the bus - doing so before might raise SCK to HIGH */
> reg = sun6i_spi_read(sspi, SUN6I_GBL_CTL_REG);
> reg |= SUN6I_GBL_CTL_BUS_ENABLE;
> @@ -701,10 +707,12 @@ static void sun6i_spi_remove(struct platform_device *pdev)
>
> static const struct sun6i_spi_cfg sun6i_a31_spi_cfg = {
> .fifo_depth = SUN6I_FIFO_DEPTH,
> + .has_clk_ctl = true,
> };
>
> static const struct sun6i_spi_cfg sun8i_h3_spi_cfg = {
> .fifo_depth = SUN8I_FIFO_DEPTH,
> + .has_clk_ctl = true,
> };
>
> static const struct of_device_id sun6i_spi_match[] = {
On Sat, 6 May 2023 10:30:10 +0300
Maksim Kiselev <[email protected]> wrote:
> From: Icenowy Zheng <[email protected]>
>
> As we're adding more properties to the OF match data, convert it to a
> struct now.
Confirmed to be just a refactor of the existing code.
> Signed-off-by: Icenowy Zheng <[email protected]>
Even if you don't change anything, and just send a patch on her behalf,
you need to add you own Signed-off-by: (applies to the other patches as
well). With that fixed:
Reviewed-by: Andre Przywara <[email protected]>
Cheers,
Andre
> ---
> drivers/spi/spi-sun6i.c | 32 ++++++++++++++++++++++----------
> 1 file changed, 22 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/spi/spi-sun6i.c b/drivers/spi/spi-sun6i.c
> index 7532c85a352c..01a01cd86db5 100644
> --- a/drivers/spi/spi-sun6i.c
> +++ b/drivers/spi/spi-sun6i.c
> @@ -85,6 +85,10 @@
> #define SUN6I_TXDATA_REG 0x200
> #define SUN6I_RXDATA_REG 0x300
>
> +struct sun6i_spi_cfg {
> + unsigned long fifo_depth;
> +};
> +
> struct sun6i_spi {
> struct spi_master *master;
> void __iomem *base_addr;
> @@ -99,7 +103,7 @@ struct sun6i_spi {
> const u8 *tx_buf;
> u8 *rx_buf;
> int len;
> - unsigned long fifo_depth;
> + const struct sun6i_spi_cfg *cfg;
> };
>
> static inline u32 sun6i_spi_read(struct sun6i_spi *sspi, u32 reg)
> @@ -156,7 +160,7 @@ static inline void sun6i_spi_fill_fifo(struct sun6i_spi *sspi)
> u8 byte;
>
> /* See how much data we can fit */
> - cnt = sspi->fifo_depth - sun6i_spi_get_tx_fifo_count(sspi);
> + cnt = sspi->cfg->fifo_depth - sun6i_spi_get_tx_fifo_count(sspi);
>
> len = min((int)cnt, sspi->len);
>
> @@ -289,14 +293,14 @@ static int sun6i_spi_transfer_one(struct spi_master *master,
> * the hardcoded value used in old generation of Allwinner
> * SPI controller. (See spi-sun4i.c)
> */
> - trig_level = sspi->fifo_depth / 4 * 3;
> + trig_level = sspi->cfg->fifo_depth / 4 * 3;
> } else {
> /*
> * Setup FIFO DMA request trigger level
> * We choose 1/2 of the full fifo depth, that value will
> * be used as DMA burst length.
> */
> - trig_level = sspi->fifo_depth / 2;
> + trig_level = sspi->cfg->fifo_depth / 2;
>
> if (tfr->tx_buf)
> reg |= SUN6I_FIFO_CTL_TF_DRQ_EN;
> @@ -410,9 +414,9 @@ static int sun6i_spi_transfer_one(struct spi_master *master,
> reg = SUN6I_INT_CTL_TC;
>
> if (!use_dma) {
> - if (rx_len > sspi->fifo_depth)
> + if (rx_len > sspi->cfg->fifo_depth)
> reg |= SUN6I_INT_CTL_RF_RDY;
> - if (tx_len > sspi->fifo_depth)
> + if (tx_len > sspi->cfg->fifo_depth)
> reg |= SUN6I_INT_CTL_TF_ERQ;
> }
>
> @@ -543,7 +547,7 @@ static bool sun6i_spi_can_dma(struct spi_master *master,
> * the fifo length we can just fill the fifo and wait for a single
> * irq, so don't bother setting up dma
> */
> - return xfer->len > sspi->fifo_depth;
> + return xfer->len > sspi->cfg->fifo_depth;
> }
>
> static int sun6i_spi_probe(struct platform_device *pdev)
> @@ -582,7 +586,7 @@ static int sun6i_spi_probe(struct platform_device *pdev)
> }
>
> sspi->master = master;
> - sspi->fifo_depth = (unsigned long)of_device_get_match_data(&pdev->dev);
> + sspi->cfg = of_device_get_match_data(&pdev->dev);
>
> master->max_speed_hz = 100 * 1000 * 1000;
> master->min_speed_hz = 3 * 1000;
> @@ -695,9 +699,17 @@ static void sun6i_spi_remove(struct platform_device *pdev)
> dma_release_channel(master->dma_rx);
> }
>
> +static const struct sun6i_spi_cfg sun6i_a31_spi_cfg = {
> + .fifo_depth = SUN6I_FIFO_DEPTH,
> +};
> +
> +static const struct sun6i_spi_cfg sun8i_h3_spi_cfg = {
> + .fifo_depth = SUN8I_FIFO_DEPTH,
> +};
> +
> static const struct of_device_id sun6i_spi_match[] = {
> - { .compatible = "allwinner,sun6i-a31-spi", .data = (void *)SUN6I_FIFO_DEPTH },
> - { .compatible = "allwinner,sun8i-h3-spi", .data = (void *)SUN8I_FIFO_DEPTH },
> + { .compatible = "allwinner,sun6i-a31-spi", .data = &sun6i_a31_spi_cfg },
> + { .compatible = "allwinner,sun8i-h3-spi", .data = &sun8i_h3_spi_cfg },
> {}
> };
> MODULE_DEVICE_TABLE(of, sun6i_spi_match);
On Sat, 6 May 2023 10:30:14 +0300
Maksim Kiselev <[email protected]> wrote:
Hi,
> Some boards form the MangoPi family (MQ\MQ-Dual\MQ-R) may have
> an optional SPI flash that connects to the SPI0 controller.
>
> This controller is the same for R329/D1/R528/T113s SoCs and
> should be supported by the sun50i-r329-spi driver.
>
> So let's add its DT node.
>
> Signed-off-by: Maksim Kiselev <[email protected]>
> ---
> .../boot/dts/allwinner/sunxi-d1s-t113.dtsi | 21 +++++++++++++++++++
> 1 file changed, 21 insertions(+)
>
> diff --git a/arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi b/arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi
> index 922e8e0e2c09..a52999240a8e 100644
> --- a/arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi
> +++ b/arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi
> @@ -108,6 +108,12 @@ rmii_pe_pins: rmii-pe-pins {
> function = "emac";
> };
>
> + /omit-if-no-ref/
> + spi0_pins: spi0-pins {
> + pins = "PC2", "PC3", "PC4", "PC5";
> + function = "spi0";
> + };
> +
> /omit-if-no-ref/
> uart1_pg6_pins: uart1-pg6-pins {
> pins = "PG6", "PG7";
> @@ -447,6 +453,21 @@ mmc2: mmc@4022000 {
> #size-cells = <0>;
> };
>
> + spi0: spi@4025000 {
> + compatible = "allwinner,sun20i-d1-spi",
> + "allwinner,sun50i-r329-spi";
> + reg = <0x04025000 0x300>;
The manual (and the other DTs) use 4K, so please use that here as well.
> + interrupts = <SOC_PERIPHERAL_IRQ(15) IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&ccu CLK_BUS_SPI0>, <&ccu CLK_SPI0>;
> + clock-names = "ahb", "mod";
> + dmas = <&dma 22>, <&dma 22>;
> + dma-names = "rx", "tx";
> + resets = <&ccu RST_BUS_SPI0>;
> + status = "disabled";
> + #address-cells = <1>;
> + #size-cells = <0>;
> + };
I think we should add the second SPI controller here as well, using
that DBI fallback compatible string.
Above looks correct when compared to the manual.
Thanks,
Andre
> +
> usb_otg: usb@4100000 {
> compatible = "allwinner,sun20i-d1-musb",
> "allwinner,sun8i-a33-musb";
On Sat, 6 May 2023 10:30:13 +0300
Maksim Kiselev <[email protected]> wrote:
> Allwinner D1/R528/T113s SPI has the same as R329 controllers
>
> Add compatible string for this controller
Please fold this into patch 1/6, and adjust accordingly to cover all
the combinations.
Cheers,
Andre
>
> Signed-off-by: Maksim Kiselev <[email protected]>
> ---
> .../devicetree/bindings/spi/allwinner,sun6i-a31-spi.yaml | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/spi/allwinner,sun6i-a31-spi.yaml b/Documentation/devicetree/bindings/spi/allwinner,sun6i-a31-spi.yaml
> index 2c1b8da35339..164bd6af9299 100644
> --- a/Documentation/devicetree/bindings/spi/allwinner,sun6i-a31-spi.yaml
> +++ b/Documentation/devicetree/bindings/spi/allwinner,sun6i-a31-spi.yaml
> @@ -30,6 +30,10 @@ properties:
> - allwinner,sun50i-h616-spi
> - allwinner,suniv-f1c100s-spi
> - const: allwinner,sun8i-h3-spi
> + - items:
> + - enum:
> + - allwinner,sun20i-d1-spi
> + - const: allwinner,sun50i-r329-spi
>
> reg:
> maxItems: 1
> Don't we need to set tfr->effective_speed_hz to the actually programmed
> clock rate here?
Yes, we should. Also we need to configure SPI sample mode by setting
SDM and SDC bits in the SPI_TCR register.
Please check a new version of this patch.
https://lore.kernel.org/linux-arm-kernel/[email protected]/
On 06/05/2023 14:59, Maxim Kiselev wrote:
>> Should this not be set up as a fallback compatible, per Samuel's
>> suggestion here:
>
> Ok, I'll do it in the next version.
>
>> I wonder what is the difference of DBI compatible. You refer to "helper
>> functions", which sounds like driver... do you mean some parts of SPI
>> controller?
>
> According to the D1 datasheet the SPI_DBI controller uses the same
> registers layout as the regular SPI0 controller.
> But also it has an additional DBI mode functionality. Support for this
> mode is not yet implemented.
> So there is no difference between 'sun50i-r329-spi' and
> 'sun50i-r329-spi-dbi' controllers types in the SPI driver.
>
> Maybe we should drop 'sun50i-r329-spi-dbi' compatible struct from here
> https://lore.kernel.org/lkml/[email protected]/
> for a while the DBI mode functionality will not be implemented?
You need both compatibles, but keep DBI compatible with regular one.
Best regards,
Krzysztof