[PATCH 0/2] serial: sprd: Modification of UNISOC Platform UART Driver

[PATCH 0/2] serial: sprd: Modification of UNISOC Platform UART Driver

[Wenhua Lin]

In order to be compatible with UNISOC's new UART IP, the UART driver uses
private data for adaptation. Patch1 adds UART timeout interrupt BIT17, and
the old project uses BIT13. In order to be compatible with all projects,
private data is used for adaptation. Patch2 adds a new compatible string.

Wenhua Lin (2):
  serial: sprd: Add support for sc9632
  dt-bindings: serial: Add a new compatible string for ums9632

 .../devicetree/bindings/serial/sprd-uart.yaml |  1 +
 drivers/tty/serial/sprd_serial.c              | 41 ++++++++++++++++---
 2 files changed, 37 insertions(+), 5 deletions(-)

-- 
2.34.1

[PATCH 1/2] serial: sprd: Add support for sc9632

[Wenhua Lin]

Due to the platform's new project uart ip upgrade,
the new project's timeout interrupt needs to use bit17
while other projects' timeout interrupt needs to use
bit13, using private data to adapt and be compatible
with all projects.

Signed-off-by: Wenhua Lin <Wenhua.Lin@unisoc.com>
---
 drivers/tty/serial/sprd_serial.c | 41 ++++++++++++++++++++++++++++----
 1 file changed, 36 insertions(+), 5 deletions(-)

diff --git a/drivers/tty/serial/sprd_serial.c b/drivers/tty/serial/sprd_serial.c
index 3fc54cc02a1f..882580c3cf37 100644
--- a/drivers/tty/serial/sprd_serial.c
+++ b/drivers/tty/serial/sprd_serial.c
@@ -53,10 +53,12 @@
 #define SPRD_IEN_TX_EMPTY	BIT(1)
 #define SPRD_IEN_BREAK_DETECT	BIT(7)
 #define SPRD_IEN_TIMEOUT	BIT(13)
+#define SPRD_IEN_DATA_TIMEOUT	BIT(17)
 
 /* interrupt clear register */
 #define SPRD_ICLR		0x0014
 #define SPRD_ICLR_TIMEOUT	BIT(13)
+#define SPRD_ICLR_DATA_TIMEOUT	BIT(17)
 
 /* line control register */
 #define SPRD_LCR		0x0018
@@ -102,6 +104,7 @@
 #define SPRD_IMSR_TX_FIFO_EMPTY	BIT(1)
 #define SPRD_IMSR_BREAK_DETECT	BIT(7)
 #define SPRD_IMSR_TIMEOUT	BIT(13)
+#define SPRD_IMSR_DATA_TIMEOUT	BIT(17)
 #define SPRD_DEFAULT_SOURCE_CLK	26000000
 
 #define SPRD_RX_DMA_STEP	1
@@ -118,6 +121,12 @@ struct sprd_uart_dma {
 	bool enable;
 };
 
+struct sprd_uart_data {
+	unsigned int timeout_ien;
+	unsigned int timeout_iclr;
+	unsigned int timeout_imsr;
+};
+
 struct sprd_uart_port {
 	struct uart_port port;
 	char name[16];
@@ -126,6 +135,7 @@ struct sprd_uart_port {
 	struct sprd_uart_dma rx_dma;
 	dma_addr_t pos;
 	unsigned char *rx_buf_tail;
+	const struct sprd_uart_data *pdata;
 };
 
 static struct sprd_uart_port *sprd_port[UART_NR_MAX];
@@ -134,6 +144,18 @@ static int sprd_ports_num;
 static int sprd_start_dma_rx(struct uart_port *port);
 static int sprd_tx_dma_config(struct uart_port *port);
 
+static const struct sprd_uart_data sc9836_data = {
+	.timeout_ien = SPRD_IEN_TIMEOUT,
+	.timeout_iclr = SPRD_ICLR_TIMEOUT,
+	.timeout_imsr = SPRD_IMSR_TIMEOUT,
+};
+
+static const struct sprd_uart_data sc9632_data = {
+	.timeout_ien = SPRD_IEN_DATA_TIMEOUT,
+	.timeout_iclr = SPRD_ICLR_DATA_TIMEOUT,
+	.timeout_imsr = SPRD_IMSR_DATA_TIMEOUT,
+};
+
 static inline unsigned int serial_in(struct uart_port *port,
 				     unsigned int offset)
 {
@@ -637,6 +659,8 @@ static irqreturn_t sprd_handle_irq(int irq, void *dev_id)
 {
 	struct uart_port *port = dev_id;
 	unsigned int ims;
+	struct sprd_uart_port *sp =
+		container_of(port, struct sprd_uart_port, port);
 
 	uart_port_lock(port);
 
@@ -647,14 +671,14 @@ static irqreturn_t sprd_handle_irq(int irq, void *dev_id)
 		return IRQ_NONE;
 	}
 
-	if (ims & SPRD_IMSR_TIMEOUT)
-		serial_out(port, SPRD_ICLR, SPRD_ICLR_TIMEOUT);
+	if (ims & sp->pdata->timeout_imsr)
+		serial_out(port, SPRD_ICLR, sp->pdata->timeout_iclr);
 
 	if (ims & SPRD_IMSR_BREAK_DETECT)
 		serial_out(port, SPRD_ICLR, SPRD_IMSR_BREAK_DETECT);
 
 	if (ims & (SPRD_IMSR_RX_FIFO_FULL | SPRD_IMSR_BREAK_DETECT |
-		   SPRD_IMSR_TIMEOUT))
+		   sp->pdata->timeout_imsr))
 		sprd_rx(port);
 
 	if (ims & SPRD_IMSR_TX_FIFO_EMPTY)
@@ -729,7 +753,7 @@ static int sprd_startup(struct uart_port *port)
 	/* enable interrupt */
 	uart_port_lock_irqsave(port, &flags);
 	ien = serial_in(port, SPRD_IEN);
-	ien |= SPRD_IEN_BREAK_DETECT | SPRD_IEN_TIMEOUT;
+	ien |= SPRD_IEN_BREAK_DETECT | sp->pdata->timeout_ien;
 	if (!sp->rx_dma.enable)
 		ien |= SPRD_IEN_RX_FULL;
 	serial_out(port, SPRD_IEN, ien);
@@ -1184,6 +1208,12 @@ static int sprd_probe(struct platform_device *pdev)
 
 	up->mapbase = res->start;
 
+	sport->pdata = of_device_get_match_data(&pdev->dev);
+	if (!sport->pdata) {
+		dev_err(&pdev->dev, "get match data failed!\n");
+		return -EINVAL;
+	}
+
 	irq = platform_get_irq(pdev, 0);
 	if (irq < 0)
 		return irq;
@@ -1248,7 +1278,8 @@ static int sprd_resume(struct device *dev)
 static SIMPLE_DEV_PM_OPS(sprd_pm_ops, sprd_suspend, sprd_resume);
 
 static const struct of_device_id serial_ids[] = {
-	{.compatible = "sprd,sc9836-uart",},
+	{.compatible = "sprd,sc9836-uart", .data = &sc9836_data},
+	{.compatible = "sprd,sc9632-uart", .data = &sc9632_data},
 	{}
 };
 MODULE_DEVICE_TABLE(of, serial_ids);
-- 
2.34.1

[PATCH 2/2] dt-bindings: serial: Add a new compatible string for ums9632

[Wenhua Lin]

The UMS9632 uses the SC9632 serial device.

Signed-off-by: Wenhua Lin <Wenhua.Lin@unisoc.com>
---
 Documentation/devicetree/bindings/serial/sprd-uart.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/serial/sprd-uart.yaml b/Documentation/devicetree/bindings/serial/sprd-uart.yaml
index f4dbb6dc2b6e..a2a5056eba04 100644
--- a/Documentation/devicetree/bindings/serial/sprd-uart.yaml
+++ b/Documentation/devicetree/bindings/serial/sprd-uart.yaml
@@ -17,6 +17,7 @@ properties:
     oneOf:
       - items:
           - enum:
+              - sprd,sc9632-uart
               - sprd,sc9860-uart
               - sprd,sc9863a-uart
               - sprd,ums512-uart
-- 
2.34.1

Re: [PATCH 2/2] dt-bindings: serial: Add a new compatible string for ums9632

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 219 bytes --]

On Wed, Nov 13, 2024 at 07:05:16PM +0800, Wenhua Lin wrote:
> The UMS9632 uses the SC9632 serial device.
> 
> Signed-off-by: Wenhua Lin <Wenhua.Lin@unisoc.com>

Acked-by: Conor Dooley <conor.dooley@microchip.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 1/2] serial: sprd: Add support for sc9632

[Baolin Wang]

On 2024/11/13 19:05, Wenhua Lin wrote:
> Due to the platform's new project uart ip upgrade,
> the new project's timeout interrupt needs to use bit17
> while other projects' timeout interrupt needs to use
> bit13, using private data to adapt and be compatible
> with all projects.
> 
> Signed-off-by: Wenhua Lin <Wenhua.Lin@unisoc.com>
> ---
>   drivers/tty/serial/sprd_serial.c | 41 ++++++++++++++++++++++++++++----
>   1 file changed, 36 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/tty/serial/sprd_serial.c b/drivers/tty/serial/sprd_serial.c
> index 3fc54cc02a1f..882580c3cf37 100644
> --- a/drivers/tty/serial/sprd_serial.c
> +++ b/drivers/tty/serial/sprd_serial.c
> @@ -53,10 +53,12 @@
>   #define SPRD_IEN_TX_EMPTY	BIT(1)
>   #define SPRD_IEN_BREAK_DETECT	BIT(7)
>   #define SPRD_IEN_TIMEOUT	BIT(13)
> +#define SPRD_IEN_DATA_TIMEOUT	BIT(17)

I don't know the meaning of 'DATA' in the new macro name. But I have no 
better name now:) Otherwise look good to me.
Reviewed-by: Baolin Wang <baolin.wang@linux.alibaba.com>

Re: [PATCH 1/2] serial: sprd: Add support for sc9632

[wenhua lin]

On Fri, Nov 15, 2024 at 1:47 PM Baolin Wang
<baolin.wang@linux.alibaba.com> wrote:
>
>
>
> On 2024/11/13 19:05, Wenhua Lin wrote:
> > Due to the platform's new project uart ip upgrade,
> > the new project's timeout interrupt needs to use bit17
> > while other projects' timeout interrupt needs to use
> > bit13, using private data to adapt and be compatible
> > with all projects.
> >
> > Signed-off-by: Wenhua Lin <Wenhua.Lin@unisoc.com>
> > ---
> >   drivers/tty/serial/sprd_serial.c | 41 ++++++++++++++++++++++++++++----
> >   1 file changed, 36 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/tty/serial/sprd_serial.c b/drivers/tty/serial/sprd_serial.c
> > index 3fc54cc02a1f..882580c3cf37 100644
> > --- a/drivers/tty/serial/sprd_serial.c
> > +++ b/drivers/tty/serial/sprd_serial.c
> > @@ -53,10 +53,12 @@
> >   #define SPRD_IEN_TX_EMPTY   BIT(1)
> >   #define SPRD_IEN_BREAK_DETECT       BIT(7)
> >   #define SPRD_IEN_TIMEOUT    BIT(13)
> > +#define SPRD_IEN_DATA_TIMEOUT        BIT(17)
>
> I don't know the meaning of 'DATA' in the new macro name. But I have no
> better name now:) Otherwise look good to me.
> Reviewed-by: Baolin Wang <baolin.wang@linux.alibaba.com>

Hi baolin:
  TIMEOUT means only timeout, DATA_TIMEOUT means timeout and fifo is not empty.
  Therefore, the macro name is distinguished by adding DATA.

Thanks

Re: [PATCH 1/2] serial: sprd: Add support for sc9632

[Baolin Wang]

On 2024/11/15 15:46, wenhua lin wrote:
> On Fri, Nov 15, 2024 at 1:47 PM Baolin Wang
> <baolin.wang@linux.alibaba.com> wrote:
>>
>>
>>
>> On 2024/11/13 19:05, Wenhua Lin wrote:
>>> Due to the platform's new project uart ip upgrade,
>>> the new project's timeout interrupt needs to use bit17
>>> while other projects' timeout interrupt needs to use
>>> bit13, using private data to adapt and be compatible
>>> with all projects.
>>>
>>> Signed-off-by: Wenhua Lin <Wenhua.Lin@unisoc.com>
>>> ---
>>>    drivers/tty/serial/sprd_serial.c | 41 ++++++++++++++++++++++++++++----
>>>    1 file changed, 36 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/tty/serial/sprd_serial.c b/drivers/tty/serial/sprd_serial.c
>>> index 3fc54cc02a1f..882580c3cf37 100644
>>> --- a/drivers/tty/serial/sprd_serial.c
>>> +++ b/drivers/tty/serial/sprd_serial.c
>>> @@ -53,10 +53,12 @@
>>>    #define SPRD_IEN_TX_EMPTY   BIT(1)
>>>    #define SPRD_IEN_BREAK_DETECT       BIT(7)
>>>    #define SPRD_IEN_TIMEOUT    BIT(13)
>>> +#define SPRD_IEN_DATA_TIMEOUT        BIT(17)
>>
>> I don't know the meaning of 'DATA' in the new macro name. But I have no
>> better name now:) Otherwise look good to me.
>> Reviewed-by: Baolin Wang <baolin.wang@linux.alibaba.com>
> 
> Hi baolin:
>    TIMEOUT means only timeout, DATA_TIMEOUT means timeout and fifo is not empty.
>    Therefore, the macro name is distinguished by adding DATA.

Good. These information should be added into commit message.

Re: [PATCH 1/2] serial: sprd: Add support for sc9632

[cixi.geng]

2024年11月13日 19:05, "Wenhua Lin" <Wenhua.Lin@unisoc.com> 写到:



> 
> Due to the platform's new project uart ip upgrade,
> 
> the new project's timeout interrupt needs to use bit17
> 
> while other projects' timeout interrupt needs to use
> 
> bit13, using private data to adapt and be compatible
> 
> with all projects.
> 
> Signed-off-by: Wenhua Lin <Wenhua.Lin@unisoc.com>
> 
> ---
> 
>  drivers/tty/serial/sprd_serial.c | 41 ++++++++++++++++++++++++++++----
> 
>  1 file changed, 36 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/tty/serial/sprd_serial.c b/drivers/tty/serial/sprd_serial.c
> 
> index 3fc54cc02a1f..882580c3cf37 100644
> 
> --- a/drivers/tty/serial/sprd_serial.c
> 
> +++ b/drivers/tty/serial/sprd_serial.c
> 
> @@ -53,10 +53,12 @@
> 
>  #define SPRD_IEN_TX_EMPTY BIT(1)
> 
>  #define SPRD_IEN_BREAK_DETECT BIT(7)
> 
>  #define SPRD_IEN_TIMEOUT BIT(13)
> 
> +#define SPRD_IEN_DATA_TIMEOUT BIT(17)
> 
>  
> 
>  /* interrupt clear register */
> 
>  #define SPRD_ICLR 0x0014
> 
>  #define SPRD_ICLR_TIMEOUT BIT(13)
> 
> +#define SPRD_ICLR_DATA_TIMEOUT BIT(17)
> 
>  
> 
>  /* line control register */
> 
>  #define SPRD_LCR 0x0018
> 
> @@ -102,6 +104,7 @@
> 
>  #define SPRD_IMSR_TX_FIFO_EMPTY BIT(1)
> 
>  #define SPRD_IMSR_BREAK_DETECT BIT(7)
> 
>  #define SPRD_IMSR_TIMEOUT BIT(13)
> 
> +#define SPRD_IMSR_DATA_TIMEOUT BIT(17)
> 
>  #define SPRD_DEFAULT_SOURCE_CLK 26000000
> 
>  
> 
>  #define SPRD_RX_DMA_STEP 1
> 
> @@ -118,6 +121,12 @@ struct sprd_uart_dma {
> 
>  bool enable;
> 
>  };
> 
>  
> 
> +struct sprd_uart_data {
> 
> + unsigned int timeout_ien;
> 
> + unsigned int timeout_iclr;
> 
> + unsigned int timeout_imsr;
> 
> +};
> 
> +
> 
>  struct sprd_uart_port {
> 
>  struct uart_port port;
> 
>  char name[16];
> 
> @@ -126,6 +135,7 @@ struct sprd_uart_port {
> 
>  struct sprd_uart_dma rx_dma;
> 
>  dma_addr_t pos;
> 
>  unsigned char *rx_buf_tail;
> 
> + const struct sprd_uart_data *pdata;
> 
>  };
> 
>  
> 
>  static struct sprd_uart_port *sprd_port[UART_NR_MAX];
> 
> @@ -134,6 +144,18 @@ static int sprd_ports_num;
> 
>  static int sprd_start_dma_rx(struct uart_port *port);
> 
>  static int sprd_tx_dma_config(struct uart_port *port);
> 
>  
> 
> +static const struct sprd_uart_data sc9836_data = {
> 
> + .timeout_ien = SPRD_IEN_TIMEOUT,
> 
> + .timeout_iclr = SPRD_ICLR_TIMEOUT,
> 
> + .timeout_imsr = SPRD_IMSR_TIMEOUT,
> 
> +};
> 
> +
> 
> +static const struct sprd_uart_data sc9632_data = {
> 
> + .timeout_ien = SPRD_IEN_DATA_TIMEOUT,
> 
> + .timeout_iclr = SPRD_ICLR_DATA_TIMEOUT,
> 
> + .timeout_imsr = SPRD_IMSR_DATA_TIMEOUT,
> 
> +};
> 
> +
> 
>  static inline unsigned int serial_in(struct uart_port *port,
> 
>  unsigned int offset)
> 
>  {
> 
> @@ -637,6 +659,8 @@ static irqreturn_t sprd_handle_irq(int irq, void *dev_id)
> 
>  {
> 
>  struct uart_port *port = dev_id;
> 
>  unsigned int ims;
> 
> + struct sprd_uart_port *sp =
> 
> + container_of(port, struct sprd_uart_port, port);
> 
>  
> 
>  uart_port_lock(port);
> 
>  
> 
> @@ -647,14 +671,14 @@ static irqreturn_t sprd_handle_irq(int irq, void *dev_id)
> 
>  return IRQ_NONE;
> 
>  }
> 
>  
> 
> - if (ims & SPRD_IMSR_TIMEOUT)
> 
> - serial_out(port, SPRD_ICLR, SPRD_ICLR_TIMEOUT);
> 
> + if (ims & sp->pdata->timeout_imsr)
> 
> + serial_out(port, SPRD_ICLR, sp->pdata->timeout_iclr);
> 
>  
> 
>  if (ims & SPRD_IMSR_BREAK_DETECT)
> 
>  serial_out(port, SPRD_ICLR, SPRD_IMSR_BREAK_DETECT);
> 
>  
> 
>  if (ims & (SPRD_IMSR_RX_FIFO_FULL | SPRD_IMSR_BREAK_DETECT |
> 
> - SPRD_IMSR_TIMEOUT))
> 
> + sp->pdata->timeout_imsr))
> 
>  sprd_rx(port);
> 
>  
> 
>  if (ims & SPRD_IMSR_TX_FIFO_EMPTY)
> 
> @@ -729,7 +753,7 @@ static int sprd_startup(struct uart_port *port)
> 
>  /* enable interrupt */
> 
>  uart_port_lock_irqsave(port, &flags);
> 
>  ien = serial_in(port, SPRD_IEN);
> 
> - ien |= SPRD_IEN_BREAK_DETECT | SPRD_IEN_TIMEOUT;
> 
> + ien |= SPRD_IEN_BREAK_DETECT | sp->pdata->timeout_ien;
> 
>  if (!sp->rx_dma.enable)
> 
>  ien |= SPRD_IEN_RX_FULL;
> 
>  serial_out(port, SPRD_IEN, ien);
> 
> @@ -1184,6 +1208,12 @@ static int sprd_probe(struct platform_device *pdev)
> 
>  
> 
>  up->mapbase = res->start;
> 
>  
> 
> + sport->pdata = of_device_get_match_data(&pdev->dev);
> 
> + if (!sport->pdata) {
> 
> + dev_err(&pdev->dev, "get match data failed!\n");
> 
> + return -EINVAL;
> 
> + }
> 
> +
> 
>  irq = platform_get_irq(pdev, 0);
> 
>  if (irq < 0)
> 
>  return irq;
> 
> @@ -1248,7 +1278,8 @@ static int sprd_resume(struct device *dev)
> 
>  static SIMPLE_DEV_PM_OPS(sprd_pm_ops, sprd_suspend, sprd_resume);
> 
>  
> 
>  static const struct of_device_id serial_ids[] = {
> 
> - {.compatible = "sprd,sc9836-uart",},
> 
> + {.compatible = "sprd,sc9836-uart", .data = &sc9836_data},
> 
> + {.compatible = "sprd,sc9632-uart", .data = &sc9632_data},
> 
>  {}
> 
>  };
> 
>  MODULE_DEVICE_TABLE(of, serial_ids);
> 
> -- 
> 
> 2.34.1
>
Acked-by: Cixi Geng <cixi.geng@linux.dev>

Re: [PATCH 2/2] dt-bindings: serial: Add a new compatible string for ums9632

[cixi.geng]

2024年11月13日 19:05, "Wenhua Lin" <Wenhua.Lin@unisoc.com> 写到:



> 
> The UMS9632 uses the SC9632 serial device.
> 
> Signed-off-by: Wenhua Lin <Wenhua.Lin@unisoc.com>
> 
> ---
> 
>  Documentation/devicetree/bindings/serial/sprd-uart.yaml | 1 +
> 
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/serial/sprd-uart.yaml b/Documentation/devicetree/bindings/serial/sprd-uart.yaml
> 
> index f4dbb6dc2b6e..a2a5056eba04 100644
> 
> --- a/Documentation/devicetree/bindings/serial/sprd-uart.yaml
> 
> +++ b/Documentation/devicetree/bindings/serial/sprd-uart.yaml
> 
> @@ -17,6 +17,7 @@ properties:
> 
>  oneOf:
> 
>  - items:
> 
>  - enum:
> 
> + - sprd,sc9632-uart
> 
>  - sprd,sc9860-uart
> 
>  - sprd,sc9863a-uart
> 
>  - sprd,ums512-uart
> 
> -- 
> 
> 2.34.1
>
Acked-by: Cixi Geng <cixi.geng@linux.dev>

Re: [PATCH 2/2] dt-bindings: serial: Add a new compatible string for ums9632

[Stanislav Jakubek]

Correct me if I'm wrong, but this patch seems incorrect to me.
The 1st patch suggets that the sc9632-uart is incompatible with sc9836-uart,
but here you make it fallback to it anyway.

Also, both of the patches seem to have made it to linux-next without the
reviews/Acks from maintainers. Maybe Greg was a bit too fast here :)

Regards,
Stanislav

Re: [PATCH 2/2] dt-bindings: serial: Add a new compatible string for ums9632

[Krzysztof Kozlowski]

On 21/11/2024 13:26, Stanislav Jakubek wrote:
> Correct me if I'm wrong, but this patch seems incorrect to me.
> The 1st patch suggets that the sc9632-uart is incompatible with sc9836-uart,
> but here you make it fallback to it anyway.
> 
> Also, both of the patches seem to have made it to linux-next without the
> reviews/Acks from maintainers. Maybe Greg was a bit too fast here :)

Yeah, this looks odd and considering totally empty commit msg (nothing
useful there), it looks like wrong choice.

Please explain the compatibility aspects. In the future: you have entire
commit msg to describe the hardware, instead of repeating the obvious -
what is visible from the diff.

Best regards,
Krzysztof

Re: [PATCH 2/2] dt-bindings: serial: Add a new compatible string for ums9632

[wenhua lin]

On Thu, Nov 21, 2024 at 8:26 PM Stanislav Jakubek
<stano.jakubek@gmail.com> wrote:
>
> Correct me if I'm wrong, but this patch seems incorrect to me.
> The 1st patch suggets that the sc9632-uart is incompatible with sc9836-uart,
> but here you make it fallback to it anyway.
>
> Also, both of the patches seem to have made it to linux-next without the
> reviews/Acks from maintainers. Maybe Greg was a bit too fast here :)
>
> Regards,
> Stanislav

Hi Stanislav:
   Thank you very much for your review, we will correct it in patch v2.
Thanks

Re: [PATCH 2/2] dt-bindings: serial: Add a new compatible string for ums9632

[wenhua lin]

On Thu, Nov 21, 2024 at 9:34 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 21/11/2024 13:26, Stanislav Jakubek wrote:
> > Correct me if I'm wrong, but this patch seems incorrect to me.
> > The 1st patch suggets that the sc9632-uart is incompatible with sc9836-uart,
> > but here you make it fallback to it anyway.
> >
> > Also, both of the patches seem to have made it to linux-next without the
> > reviews/Acks from maintainers. Maybe Greg was a bit too fast here :)
>
> Yeah, this looks odd and considering totally empty commit msg (nothing
> useful there), it looks like wrong choice.
>
> Please explain the compatibility aspects. In the future: you have entire
> commit msg to describe the hardware, instead of repeating the obvious -
> what is visible from the diff.
>
> Best regards,
> Krzysztof

Hi Krzysztof:
   Thank you very much for your review, we will correct it in patch v2.
Thanks

Re: [PATCH 2/2] dt-bindings: serial: Add a new compatible string for ums9632

[Krzysztof Kozlowski]

On 13/01/2025 03:29, wenhua lin wrote:
> On Thu, Nov 21, 2024 at 9:34 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>
>> On 21/11/2024 13:26, Stanislav Jakubek wrote:
>>> Correct me if I'm wrong, but this patch seems incorrect to me.
>>> The 1st patch suggets that the sc9632-uart is incompatible with sc9836-uart,
>>> but here you make it fallback to it anyway.
>>>
>>> Also, both of the patches seem to have made it to linux-next without the
>>> reviews/Acks from maintainers. Maybe Greg was a bit too fast here :)
>>
>> Yeah, this looks odd and considering totally empty commit msg (nothing
>> useful there), it looks like wrong choice.
>>
>> Please explain the compatibility aspects. In the future: you have entire
>> commit msg to describe the hardware, instead of repeating the obvious -
>> what is visible from the diff.
>>
>> Best regards,
>> Krzysztof
> 
> Hi Krzysztof:
>    Thank you very much for your review, we will correct it in patch v2.
I wrote - commit msg, not changelog. Sigh...

Best regards,
Krzysztof