[PATCH v4 0/6] Support ROCKCHIP SPI new feature

[PATCH v4 0/6] Support ROCKCHIP SPI new feature

[Jon Lin]

Changes in v4:
- Adjust the order patches
- Simply commit massage like redundancy "application" content

Changes in v3:
- Fix compile error which is find by Sascha in [v2,2/8]

Jon Lin (6):
  dt-bindings: spi: spi-rockchip: add description for rv1126 and rk3568
  spi: rockchip: add compatible string for rv1126 and rk3568
  spi: rockchip: Set rx_fifo interrupt waterline base on transfer item
  spi: rockchip: Wait for STB status in slave mode tx_xfer
  spi: rockchip: Support cs-gpio
  spi: rockchip: Support SPI_CS_HIGH

 .../devicetree/bindings/spi/spi-rockchip.yaml |  2 +
 drivers/spi/spi-rockchip.c                    | 96 +++++++++++++++----
 2 files changed, 82 insertions(+), 16 deletions(-)

-- 
2.17.1




_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 1/6] dt-bindings: spi: spi-rockchip: add description for rv1126 and rk3568

[Jon Lin]

The description below will be used for rv1126.dtsi or rk3568.dtsi in
the future

Signed-off-by: Jon Lin <jon.lin@rock-chips.com>
---

Changes in v4:
- Adjust the order patches
- Simply commit massage like redundancy "application" content

Changes in v3:
- Fix compile error which is find by Sascha in [v2,2/8]

 Documentation/devicetree/bindings/spi/spi-rockchip.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/spi/spi-rockchip.yaml b/Documentation/devicetree/bindings/spi/spi-rockchip.yaml
index 1e6cf29e6388..2d7957f9ae0a 100644
--- a/Documentation/devicetree/bindings/spi/spi-rockchip.yaml
+++ b/Documentation/devicetree/bindings/spi/spi-rockchip.yaml
@@ -27,12 +27,14 @@ properties:
       - items:
           - enum:
               - rockchip,px30-spi
+              - rockchip,rv1126-spi
               - rockchip,rk3188-spi
               - rockchip,rk3288-spi
               - rockchip,rk3308-spi
               - rockchip,rk3328-spi
               - rockchip,rk3368-spi
               - rockchip,rk3399-spi
+              - rockchip,rk3568-spi
           - const: rockchip,rk3066-spi
 
   reg:
-- 
2.17.1




_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 2/6] spi: rockchip: add compatible string for rv1126 and rk3568

[Jon Lin]

Add compatible string for rv1126 and rk3568 for potential applications.

Signed-off-by: Jon Lin <jon.lin@rock-chips.com>
---

Changes in v4: None
Changes in v3: None

 drivers/spi/spi-rockchip.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/spi/spi-rockchip.c b/drivers/spi/spi-rockchip.c
index 52d6259d96ed..99d66dff8c89 100644
--- a/drivers/spi/spi-rockchip.c
+++ b/drivers/spi/spi-rockchip.c
@@ -921,7 +921,9 @@ static const struct of_device_id rockchip_spi_dt_match[] = {
 	{ .compatible = "rockchip,rk3328-spi", },
 	{ .compatible = "rockchip,rk3368-spi", },
 	{ .compatible = "rockchip,rk3399-spi", },
+	{ .compatible = "rockchip,rk3568-spi", },
 	{ .compatible = "rockchip,rv1108-spi", },
+	{ .compatible = "rockchip,rv1126-spi", },
 	{ },
 };
 MODULE_DEVICE_TABLE(of, rockchip_spi_dt_match);
-- 
2.17.1




_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 3/6] spi: rockchip: Set rx_fifo interrupt waterline base on transfer item

[Jon Lin]

The error here is to calculate the width as 8 bits. In fact, 16 bits
should be considered.

Signed-off-by: Jon Lin <jon.lin@rock-chips.com>
---

Changes in v4: None
Changes in v3: None

 drivers/spi/spi-rockchip.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi-rockchip.c b/drivers/spi/spi-rockchip.c
index 99d66dff8c89..c4eff9a8a14d 100644
--- a/drivers/spi/spi-rockchip.c
+++ b/drivers/spi/spi-rockchip.c
@@ -540,8 +540,8 @@ static int rockchip_spi_config(struct rockchip_spi *rs,
 	 * interrupt exactly when the fifo is full doesn't seem to work,
 	 * so we need the strict inequality here
 	 */
-	if (xfer->len < rs->fifo_len)
-		writel_relaxed(xfer->len - 1, rs->regs + ROCKCHIP_SPI_RXFTLR);
+	if ((xfer->len / rs->n_bytes) < rs->fifo_len)
+		writel_relaxed(xfer->len / rs->n_bytes - 1, rs->regs + ROCKCHIP_SPI_RXFTLR);
 	else
 		writel_relaxed(rs->fifo_len / 2 - 1, rs->regs + ROCKCHIP_SPI_RXFTLR);
 
-- 
2.17.1




_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 4/6] spi: rockchip: Wait for STB status in slave mode tx_xfer

[Jon Lin]

After ROCKCHIP_SPI_VER2_TYPE2, SR->STB is a more accurate judgment
bit for spi slave transmition.

Signed-off-by: Jon Lin <jon.lin@rock-chips.com>
---

Changes in v4: None
Changes in v3: None

 drivers/spi/spi-rockchip.c | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/drivers/spi/spi-rockchip.c b/drivers/spi/spi-rockchip.c
index c4eff9a8a14d..4dfff91b2743 100644
--- a/drivers/spi/spi-rockchip.c
+++ b/drivers/spi/spi-rockchip.c
@@ -116,13 +116,14 @@
 #define BAUDR_SCKDV_MIN				2
 #define BAUDR_SCKDV_MAX				65534
 
-/* Bit fields in SR, 5bit */
-#define SR_MASK						0x1f
+/* Bit fields in SR, 6bit */
+#define SR_MASK						0x3f
 #define SR_BUSY						(1 << 0)
 #define SR_TF_FULL					(1 << 1)
 #define SR_TF_EMPTY					(1 << 2)
 #define SR_RF_EMPTY					(1 << 3)
 #define SR_RF_FULL					(1 << 4)
+#define SR_SLAVE_TX_BUSY				(1 << 5)
 
 /* Bit fields in ISR, IMR, ISR, RISR, 5bit */
 #define INT_MASK					0x1f
@@ -197,13 +198,19 @@ static inline void spi_enable_chip(struct rockchip_spi *rs, bool enable)
 	writel_relaxed((enable ? 1U : 0U), rs->regs + ROCKCHIP_SPI_SSIENR);
 }
 
-static inline void wait_for_idle(struct rockchip_spi *rs)
+static inline void wait_for_tx_idle(struct rockchip_spi *rs, bool slave_mode)
 {
 	unsigned long timeout = jiffies + msecs_to_jiffies(5);
 
 	do {
-		if (!(readl_relaxed(rs->regs + ROCKCHIP_SPI_SR) & SR_BUSY))
-			return;
+		if (slave_mode) {
+			if (!(readl_relaxed(rs->regs + ROCKCHIP_SPI_SR) & SR_SLAVE_TX_BUSY) &&
+			    !((readl_relaxed(rs->regs + ROCKCHIP_SPI_SR) & SR_BUSY)))
+				return;
+		} else {
+			if (!(readl_relaxed(rs->regs + ROCKCHIP_SPI_SR) & SR_BUSY))
+				return;
+		}
 	} while (!time_after(jiffies, timeout));
 
 	dev_warn(rs->dev, "spi controller is in busy state!\n");
@@ -383,7 +390,7 @@ static void rockchip_spi_dma_txcb(void *data)
 		return;
 
 	/* Wait until the FIFO data completely. */
-	wait_for_idle(rs);
+	wait_for_tx_idle(rs, ctlr->slave);
 
 	spi_enable_chip(rs, false);
 	spi_finalize_current_transfer(ctlr);
@@ -545,7 +552,7 @@ static int rockchip_spi_config(struct rockchip_spi *rs,
 	else
 		writel_relaxed(rs->fifo_len / 2 - 1, rs->regs + ROCKCHIP_SPI_RXFTLR);
 
-	writel_relaxed(rs->fifo_len / 2, rs->regs + ROCKCHIP_SPI_DMATDLR);
+	writel_relaxed(rs->fifo_len / 2 - 1, rs->regs + ROCKCHIP_SPI_DMATDLR);
 	writel_relaxed(rockchip_spi_calc_burst_size(xfer->len / rs->n_bytes) - 1,
 		       rs->regs + ROCKCHIP_SPI_DMARDLR);
 	writel_relaxed(dmacr, rs->regs + ROCKCHIP_SPI_DMACR);
-- 
2.17.1




_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 5/6] spi: rockchip: Support cs-gpio

[Jon Lin]

1.Add standard cs-gpio support
2.Refer to spi-controller.yaml for details

Signed-off-by: Jon Lin <jon.lin@rock-chips.com>
---

Changes in v4: None
Changes in v3: None

 drivers/spi/spi-rockchip.c | 61 ++++++++++++++++++++++++++++++++++----
 1 file changed, 56 insertions(+), 5 deletions(-)

diff --git a/drivers/spi/spi-rockchip.c b/drivers/spi/spi-rockchip.c
index 4dfff91b2743..be7d90e18a3f 100644
--- a/drivers/spi/spi-rockchip.c
+++ b/drivers/spi/spi-rockchip.c
@@ -6,6 +6,7 @@
 
 #include <linux/clk.h>
 #include <linux/dmaengine.h>
+#include <linux/gpio.h>
 #include <linux/interrupt.h>
 #include <linux/module.h>
 #include <linux/of.h>
@@ -157,7 +158,8 @@
  */
 #define ROCKCHIP_SPI_MAX_TRANLEN		0xffff
 
-#define ROCKCHIP_SPI_MAX_CS_NUM			2
+/* 2 for native cs, 2 for cs-gpio */
+#define ROCKCHIP_SPI_MAX_CS_NUM			4
 #define ROCKCHIP_SPI_VER2_TYPE1			0x05EC0002
 #define ROCKCHIP_SPI_VER2_TYPE2			0x00110002
 
@@ -191,6 +193,7 @@ struct rockchip_spi {
 	bool cs_asserted[ROCKCHIP_SPI_MAX_CS_NUM];
 
 	bool slave_abort;
+	bool gpio_requested;
 };
 
 static inline void spi_enable_chip(struct rockchip_spi *rs, bool enable)
@@ -245,11 +248,15 @@ static void rockchip_spi_set_cs(struct spi_device *spi, bool enable)
 		/* Keep things powered as long as CS is asserted */
 		pm_runtime_get_sync(rs->dev);
 
-		ROCKCHIP_SPI_SET_BITS(rs->regs + ROCKCHIP_SPI_SER,
-				      BIT(spi->chip_select));
+		if (gpio_is_valid(spi->cs_gpio))
+			ROCKCHIP_SPI_SET_BITS(rs->regs + ROCKCHIP_SPI_SER, 1);
+		else
+			ROCKCHIP_SPI_SET_BITS(rs->regs + ROCKCHIP_SPI_SER, BIT(spi->chip_select));
 	} else {
-		ROCKCHIP_SPI_CLR_BITS(rs->regs + ROCKCHIP_SPI_SER,
-				      BIT(spi->chip_select));
+		if (gpio_is_valid(spi->cs_gpio))
+			ROCKCHIP_SPI_CLR_BITS(rs->regs + ROCKCHIP_SPI_SER, 1);
+		else
+			ROCKCHIP_SPI_CLR_BITS(rs->regs + ROCKCHIP_SPI_SER, BIT(spi->chip_select));
 
 		/* Drop reference from when we first asserted CS */
 		pm_runtime_put(rs->dev);
@@ -632,6 +639,47 @@ static bool rockchip_spi_can_dma(struct spi_controller *ctlr,
 	return xfer->len / bytes_per_word >= rs->fifo_len;
 }
 
+static int rockchip_spi_setup(struct spi_device *spi)
+{
+
+	int ret = -EINVAL;
+	struct rockchip_spi *rs = spi_controller_get_devdata(spi->controller);
+
+	if (spi->cs_gpio == -ENOENT)
+		return 0;
+
+	if (!rs->gpio_requested && gpio_is_valid(spi->cs_gpio)) {
+		ret = gpio_request_one(spi->cs_gpio,
+				       (spi->mode & SPI_CS_HIGH) ?
+				       GPIOF_OUT_INIT_LOW : GPIOF_OUT_INIT_HIGH,
+				       dev_name(&spi->dev));
+		if (ret)
+			dev_err(&spi->dev, "can't request chipselect gpio %d\n",
+				spi->cs_gpio);
+		else
+			rs->gpio_requested = true;
+	} else {
+		if (gpio_is_valid(spi->cs_gpio)) {
+			int mode = ((spi->mode & SPI_CS_HIGH) ? 0 : 1);
+
+			ret = gpio_direction_output(spi->cs_gpio, mode);
+			if (ret)
+				dev_err(&spi->dev, "chipselect gpio %d setup failed (%d)\n",
+					spi->cs_gpio, ret);
+		}
+	}
+
+	return ret;
+}
+
+static void rockchip_spi_cleanup(struct spi_device *spi)
+{
+	struct rockchip_spi *rs = spi_controller_get_devdata(spi->controller);
+
+	if (rs->gpio_requested)
+		gpio_free(spi->cs_gpio);
+}
+
 static int rockchip_spi_probe(struct platform_device *pdev)
 {
 	int ret;
@@ -706,6 +754,7 @@ static int rockchip_spi_probe(struct platform_device *pdev)
 
 	rs->dev = &pdev->dev;
 	rs->freq = clk_get_rate(rs->spiclk);
+	rs->gpio_requested = false;
 
 	if (!of_property_read_u32(pdev->dev.of_node, "rx-sample-delay-ns",
 				  &rsd_nsecs)) {
@@ -759,6 +808,8 @@ static int rockchip_spi_probe(struct platform_device *pdev)
 	ctlr->max_speed_hz = min(rs->freq / BAUDR_SCKDV_MIN, MAX_SCLK_OUT);
 
 	ctlr->set_cs = rockchip_spi_set_cs;
+	ctlr->setup = rockchip_spi_setup;
+	ctlr->cleanup = rockchip_spi_cleanup;
 	ctlr->transfer_one = rockchip_spi_transfer_one;
 	ctlr->max_transfer_size = rockchip_spi_max_transfer_size;
 	ctlr->handle_err = rockchip_spi_handle_err;
-- 
2.17.1




_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 6/6] spi: rockchip: Support SPI_CS_HIGH

[Jon Lin]

1.Add standard spi-cs-high support
2.Refer to spi-controller.yaml for details

Signed-off-by: Jon Lin <jon.lin@rock-chips.com>
---

Changes in v4: None
Changes in v3: None

 drivers/spi/spi-rockchip.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi-rockchip.c b/drivers/spi/spi-rockchip.c
index be7d90e18a3f..4b08be25c33d 100644
--- a/drivers/spi/spi-rockchip.c
+++ b/drivers/spi/spi-rockchip.c
@@ -108,6 +108,8 @@
 #define CR0_OPM_MASTER				0x0
 #define CR0_OPM_SLAVE				0x1
 
+#define CR0_SOI_OFFSET				23
+
 #define CR0_MTM_OFFSET				0x21
 
 /* Bit fields in SER, 2bit */
@@ -238,7 +240,7 @@ static void rockchip_spi_set_cs(struct spi_device *spi, bool enable)
 {
 	struct spi_controller *ctlr = spi->controller;
 	struct rockchip_spi *rs = spi_controller_get_devdata(ctlr);
-	bool cs_asserted = !enable;
+	bool cs_asserted = spi->mode & SPI_CS_HIGH ? enable : !enable;
 
 	/* Return immediately for no-op */
 	if (cs_asserted == rs->cs_asserted[spi->chip_select])
@@ -509,6 +511,8 @@ static int rockchip_spi_config(struct rockchip_spi *rs,
 	cr0 |= (spi->mode & 0x3U) << CR0_SCPH_OFFSET;
 	if (spi->mode & SPI_LSB_FIRST)
 		cr0 |= CR0_FBM_LSB << CR0_FBM_OFFSET;
+	if (spi->mode & SPI_CS_HIGH)
+		cr0 |= BIT(spi->chip_select) << CR0_SOI_OFFSET;
 
 	if (xfer->rx_buf && xfer->tx_buf)
 		cr0 |= CR0_XFM_TR << CR0_XFM_OFFSET;
@@ -787,7 +791,7 @@ static int rockchip_spi_probe(struct platform_device *pdev)
 
 	ctlr->auto_runtime_pm = true;
 	ctlr->bus_num = pdev->id;
-	ctlr->mode_bits = SPI_CPOL | SPI_CPHA | SPI_LOOP | SPI_LSB_FIRST;
+	ctlr->mode_bits = SPI_CPOL | SPI_CPHA | SPI_LOOP | SPI_LSB_FIRST | SPI_CS_HIGH;
 	if (slave_mode) {
 		ctlr->mode_bits |= SPI_NO_CS;
 		ctlr->slave_abort = rockchip_spi_slave_abort;
-- 
2.17.1




_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v4 1/6] dt-bindings: spi: spi-rockchip: add description for rv1126 and rk3568

[Johan Jonker]

Hi Jon,

On 6/7/21 8:34 AM, Jon Lin wrote:
> The description below will be used for rv1126.dtsi or rk3568.dtsi in
> the future
> 
> Signed-off-by: Jon Lin <jon.lin@rock-chips.com>
> ---
> 
> Changes in v4:
> - Adjust the order patches
> - Simply commit massage like redundancy "application" content
> 
> Changes in v3:
> - Fix compile error which is find by Sascha in [v2,2/8]
> 
>  Documentation/devicetree/bindings/spi/spi-rockchip.yaml | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/spi/spi-rockchip.yaml b/Documentation/devicetree/bindings/spi/spi-rockchip.yaml
> index 1e6cf29e6388..2d7957f9ae0a 100644
> --- a/Documentation/devicetree/bindings/spi/spi-rockchip.yaml
> +++ b/Documentation/devicetree/bindings/spi/spi-rockchip.yaml
> @@ -27,12 +27,14 @@ properties:
>        - items:
>            - enum:
>                - rockchip,px30-spi

> +              - rockchip,rv1126-spi

This list is sort alphabetically.
Move "rockchip,rv1126-spi" below "rockchip,rk3568-spi"

>                - rockchip,rk3188-spi
>                - rockchip,rk3288-spi
>                - rockchip,rk3308-spi
>                - rockchip,rk3328-spi
>                - rockchip,rk3368-spi
>                - rockchip,rk3399-spi
> +              - rockchip,rk3568-spi


>            - const: rockchip,rk3066-spi
>  
>    reg:
> 

===

Your comment in [PATCH v3 3/8]:
>> Adding "rockchip,rv1126-spi" to rockchip_spi_dt_match[] is strictly not
>> needed when using "rockchip,rk3066-spi" as fall back string.
>> Could a maintainer advise?
>>
>> Maybe this bug of mine should revert too?? Or is it legacy?
>> spi: rockchip: add compatible string for px30 rk3308 rk3328
>> https://lore.kernel.org/r/20200309151004.7780-1-jbx6244@gmail.com

> I agree with you. If the maintainer doesn't have any comments, I will use
> "rockchip,spi" as compatible names for the subsequent rk platform.

Compatibility strings are supposed to be SoC orientated.
So generic ones like in the manufacturer tree can't be used here.

===

Johan

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v4 1/6] dt-bindings: spi: spi-rockchip: add description for rv1126 and rk3568

[Heiko Stübner]

Am Montag, 7. Juni 2021, 10:15:30 CEST schrieb Johan Jonker:
> Hi Jon,
> 
> On 6/7/21 8:34 AM, Jon Lin wrote:
> > The description below will be used for rv1126.dtsi or rk3568.dtsi in
> > the future
> > 
> > Signed-off-by: Jon Lin <jon.lin@rock-chips.com>
> > ---
> > 
> > Changes in v4:
> > - Adjust the order patches
> > - Simply commit massage like redundancy "application" content
> > 
> > Changes in v3:
> > - Fix compile error which is find by Sascha in [v2,2/8]
> > 
> >  Documentation/devicetree/bindings/spi/spi-rockchip.yaml | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/spi/spi-rockchip.yaml b/Documentation/devicetree/bindings/spi/spi-rockchip.yaml
> > index 1e6cf29e6388..2d7957f9ae0a 100644
> > --- a/Documentation/devicetree/bindings/spi/spi-rockchip.yaml
> > +++ b/Documentation/devicetree/bindings/spi/spi-rockchip.yaml
> > @@ -27,12 +27,14 @@ properties:
> >        - items:
> >            - enum:
> >                - rockchip,px30-spi
> 
> > +              - rockchip,rv1126-spi
> 
> This list is sort alphabetically.
> Move "rockchip,rv1126-spi" below "rockchip,rk3568-spi"
> 
> >                - rockchip,rk3188-spi
> >                - rockchip,rk3288-spi
> >                - rockchip,rk3308-spi
> >                - rockchip,rk3328-spi
> >                - rockchip,rk3368-spi
> >                - rockchip,rk3399-spi
> > +              - rockchip,rk3568-spi
> 
> 
> >            - const: rockchip,rk3066-spi
> >  
> >    reg:
> > 
> 
> ===
> 
> Your comment in [PATCH v3 3/8]:
> >> Adding "rockchip,rv1126-spi" to rockchip_spi_dt_match[] is strictly not
> >> needed when using "rockchip,rk3066-spi" as fall back string.
> >> Could a maintainer advise?
> >>
> >> Maybe this bug of mine should revert too?? Or is it legacy?
> >> spi: rockchip: add compatible string for px30 rk3308 rk3328
> >> https://lore.kernel.org/r/20200309151004.7780-1-jbx6244@gmail.com
> 
> > I agree with you. If the maintainer doesn't have any comments, I will use
> > "rockchip,spi" as compatible names for the subsequent rk platform.
> 
> Compatibility strings are supposed to be SoC orientated.
> So generic ones like in the manufacturer tree can't be used here.

Johan ist right :-) .

rockchip,spi won't work at all, especially as these controllers always change
over time. [0]

Best example is the iommu. We started with "rockchip,iommu" thinking this
won't change over time, but with the rk3568 we get a new slightly different
iommu.

The vendor-kernel then introduces somewhat random "-vX" additions to
distinguish them, but often they do seem to be very software-centric.

Meaning, hardware-designers moved stuff around and software-developers
then invented the versioning to differentiate between versions.

The devicetree is supposed to describe the hardware though, so going with
the relevant soc-specific compatible gives us the necessary hardware-centric
differentiation.

Also this allows to catch later issues with specific soc implementations ;-)
Like 6 monts down the road we discover some special behaviour on the
rk3568 and devicetree is supposed to be stable.

So having the relevant compatibles in place allows us to just add driver
fixes and have those apply on the rk3568 if that is need at some point.

Heiko




_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v4 1/6] dt-bindings: spi: spi-rockchip: add description for rv1126 and rk3568

[Jon Lin]

On 6/7/21 5:04 PM, Heiko Stübner wrote:
> Am Montag, 7. Juni 2021, 10:15:30 CEST schrieb Johan Jonker:
>> Hi Jon,
>>
>> On 6/7/21 8:34 AM, Jon Lin wrote:
>>> The description below will be used for rv1126.dtsi or rk3568.dtsi in
>>> the future
>>>
>>> Signed-off-by: Jon Lin <jon.lin@rock-chips.com>
>>> ---
>>>
>>> Changes in v4:
>>> - Adjust the order patches
>>> - Simply commit massage like redundancy "application" content
>>>
>>> Changes in v3:
>>> - Fix compile error which is find by Sascha in [v2,2/8]
>>>
>>>   Documentation/devicetree/bindings/spi/spi-rockchip.yaml | 2 ++
>>>   1 file changed, 2 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/spi/spi-rockchip.yaml b/Documentation/devicetree/bindings/spi/spi-rockchip.yaml
>>> index 1e6cf29e6388..2d7957f9ae0a 100644
>>> --- a/Documentation/devicetree/bindings/spi/spi-rockchip.yaml
>>> +++ b/Documentation/devicetree/bindings/spi/spi-rockchip.yaml
>>> @@ -27,12 +27,14 @@ properties:
>>>         - items:
>>>             - enum:
>>>                 - rockchip,px30-spi
>>> +              - rockchip,rv1126-spi
>> This list is sort alphabetically.
>> Move "rockchip,rv1126-spi" below "rockchip,rk3568-spi"
>>
>>>                 - rockchip,rk3188-spi
>>>                 - rockchip,rk3288-spi
>>>                 - rockchip,rk3308-spi
>>>                 - rockchip,rk3328-spi
>>>                 - rockchip,rk3368-spi
>>>                 - rockchip,rk3399-spi
>>> +              - rockchip,rk3568-spi
>>
>>>             - const: rockchip,rk3066-spi
>>>   
>>>     reg:
>>>
>> ===
>>
>> Your comment in [PATCH v3 3/8]:
>>>> Adding "rockchip,rv1126-spi" to rockchip_spi_dt_match[] is strictly not
>>>> needed when using "rockchip,rk3066-spi" as fall back string.
>>>> Could a maintainer advise?
With consulting to my colleague，we plane to:
1.If new soc's spi ip is compatible with the fall back one, we wont add 
new compatible id to the code.
2.I will add new fall back string stand for new generation ip: 
rockchip,rv1126-spi
>>>>
>>>> Maybe this bug of mine should revert too?? Or is it legacy?
>>>> spi: rockchip: add compatible string for px30 rk3308 rk3328
>>>> https://lore.kernel.org/r/20200309151004.7780-1-jbx6244@gmail.com
>>> I agree with you. If the maintainer doesn't have any comments, I will use
>>> "rockchip,spi" as compatible names for the subsequent rk platform.
>> Compatibility strings are supposed to be SoC orientated.
>> So generic ones like in the manufacturer tree can't be used here.
> Johan ist right :-) .
>
> rockchip,spi won't work at all, especially as these controllers always change
> over time. [0]
>
> Best example is the iommu. We started with "rockchip,iommu" thinking this
> won't change over time, but with the rk3568 we get a new slightly different
> iommu.
>
> The vendor-kernel then introduces somewhat random "-vX" additions to
> distinguish them, but often they do seem to be very software-centric.
>
> Meaning, hardware-designers moved stuff around and software-developers
> then invented the versioning to differentiate between versions.
>
> The devicetree is supposed to describe the hardware though, so going with
> the relevant soc-specific compatible gives us the necessary hardware-centric
> differentiation.
>
> Also this allows to catch later issues with specific soc implementations ;-)
> Like 6 monts down the road we discover some special behaviour on the
> rk3568 and devicetree is supposed to be stable.
>
> So having the relevant compatibles in place allows us to just add driver
> fixes and have those apply on the rk3568 if that is need at some point.
>
> Heiko
>
After the explain from you and Johan, I found that the idea 
"rockchip,spi" was immature.
>
>
>
>



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v4 1/6] dt-bindings: spi: spi-rockchip: add description for rv1126 and rk3568

[Kever Yang]

Hi Heiko, Johan,

On 2021/6/7 下午5:04, Heiko Stübner wrote:
> Your comment in [PATCH v3 3/8]:
>>>> Adding "rockchip,rv1126-spi" to rockchip_spi_dt_match[] is strictly not
>>>> needed when using "rockchip,rk3066-spi" as fall back string.
>>>> Could a maintainer advise?
>>>>
>>>> Maybe this bug of mine should revert too?? Or is it legacy?
>>>> spi: rockchip: add compatible string for px30 rk3308 rk3328
>>>> https://lore.kernel.org/r/20200309151004.7780-1-jbx6244@gmail.com
>>> I agree with you. If the maintainer doesn't have any comments, I will use
>>> "rockchip,spi" as compatible names for the subsequent rk platform.
>> Compatibility strings are supposed to be SoC orientated.
>> So generic ones like in the manufacturer tree can't be used here.
> Johan ist right :-) .
>
> rockchip,spi won't work at all, especially as these controllers always change
> over time. [0]
>
> Best example is the iommu. We started with "rockchip,iommu" thinking this
> won't change over time, but with the rk3568 we get a new slightly different
> iommu.


Rockchip SPI and SFC controller can use a generic compatible string, 
because there is a version

register inside the IP, and all the feature update will have a new IP 
version, so the driver is

used for the SPI/SFC IP  in all SoCs, we don't need to care which SoC is 
using this driver.

If we have to use the compatible string "rockchip,rk3066-spi" and each 
for a new soc, then we

have to update the driver compatible id list and document for each soc 
which is totally not need

and not correct  to do it.

The example "iommu" is different, because there is no version register 
inside the IP and the IP

can not identify itself, which need a software define "-vX".


Thanks,

- Kever

> The vendor-kernel then introduces somewhat random "-vX" additions to
> distinguish them, but often they do seem to be very software-centric.
>
> Meaning, hardware-designers moved stuff around and software-developers
> then invented the versioning to differentiate between versions.
>
> The devicetree is supposed to describe the hardware though, so going with
> the relevant soc-specific compatible gives us the necessary hardware-centric
> differentiation.
>
> Also this allows to catch later issues with specific soc implementations ;-)
> Like 6 monts down the road we discover some special behaviour on the
> rk3568 and devicetree is supposed to be stable.
>
> So having the relevant compatibles in place allows us to just add driver
> fixes and have those apply on the rk3568 if that is need at some point.
>
> Heiko
>
>
>
>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
>
>



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel