diff mbox

[v3,3/3] serial: 8250_dw: add ACPI support for uart on Hisilicon Hip05 soc

Message ID 1468580485-28567-4-git-send-email-wangkefeng.wang@huawei.com (mailing list archive)
State Not Applicable, archived
Headers show

Commit Message

Kefeng Wang July 15, 2016, 11:01 a.m. UTC
Use built-in device properties to set device parameters for the
existing device probed by acpi.

Add ACPI identifier for UART on Hisilicon Hip05 soc, be careful
that it is not 16550 compatibal, so we need set "reg-io-width"
and "reg-shift" by _DSD method in DSDT.

Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
 drivers/tty/serial/8250/8250_dw.c | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

Comments

Kefeng Wang Aug. 8, 2016, 6:10 a.m. UTC | #1
+ kernel malilist

Hi Andriy and all, any comments, thanks.

On 2016/7/15 19:01, Kefeng Wang wrote:
> Use built-in device properties to set device parameters for the
> existing device probed by acpi.
> 
> Add ACPI identifier for UART on Hisilicon Hip05 soc, be careful
> that it is not 16550 compatibal, so we need set "reg-io-width"
> and "reg-shift" by _DSD method in DSDT.
> 
> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
> ---
>  drivers/tty/serial/8250/8250_dw.c | 23 +++++++++++++++++------
>  1 file changed, 17 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
> index d6934310..5ab9cfe 100644
> --- a/drivers/tty/serial/8250/8250_dw.c
> +++ b/drivers/tty/serial/8250/8250_dw.c
> @@ -297,12 +297,6 @@ static void dw8250_quirks(struct uart_port *p, struct dw8250_data *data)
>  			p->serial_in = dw8250_serial_in32be;
>  			p->serial_out = dw8250_serial_out32be;
>  		}
> -	} else if (has_acpi_companion(p->dev)) {
> -		p->iotype = UPIO_MEM32;
> -		p->regshift = 2;
> -		p->serial_in = dw8250_serial_in32;
> -		/* So far none of there implement the Busy Functionality */
> -		data->uart_16550_compatible = true;
>  	}
>  
>  	/* Platforms with iDMA */
> @@ -352,6 +346,19 @@ static void dw8250_setup_port(struct uart_port *p)
>  		up->capabilities |= UART_CAP_AFE;
>  }
>  
> +static struct property_entry dw8250_properties[] = {
> +	PROPERTY_ENTRY_U32("reg-io-width", 4),
> +	PROPERTY_ENTRY_U32("reg-shift", 2),
> +	PROPERTY_ENTRY_BOOL("snps,uart-16550-compatible"),
> +	{ },
> +};
> +
> +/* non 16550 compatible id list*/
> +static const struct acpi_device_id non_16550_ids[] = {
> +	{ "HISI0031", 0 },
> +	{ },
> +};
> +
>  static int dw8250_probe(struct platform_device *pdev)
>  {
>  	struct uart_8250_port uart = {};
> @@ -395,6 +402,9 @@ static int dw8250_probe(struct platform_device *pdev)
>  	if (!data)
>  		return -ENOMEM;
>  
> +	if (has_acpi_companion(dev) && !acpi_match_device(non_16550_ids, dev))
> +		platform_device_add_properties(pdev, dw8250_properties);
> +
>  	data->dma.fn = dw8250_fallback_dma_filter;
>  	data->usr_reg = DW_UART_USR;
>  	p->private_data = data;
> @@ -619,6 +629,7 @@ static const struct acpi_device_id dw8250_acpi_match[] = {
>  	{ "APMC0D08", 0},
>  	{ "AMD0020", 0 },
>  	{ "AMDI0020", 0 },
> +	{ "HISI0031", 0 },
>  	{ },
>  };
>  MODULE_DEVICE_TABLE(acpi, dw8250_acpi_match);
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Andy Shevchenko Aug. 22, 2016, 11:24 a.m. UTC | #2
On Fri, 2016-07-15 at 19:01 +0800, Kefeng Wang wrote:
> Use built-in device properties to set device parameters for the
> existing device probed by acpi.

acpi -> ACPI

> 
> Add ACPI identifier for UART on Hisilicon Hip05 soc, be careful

soc -> SoC

> that it is not 16550 compatibal, so we need set "reg-io-width"

compatibal -> compatible

> and "reg-shift" by _DSD method in DSDT.
> 
> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
> ---
>  drivers/tty/serial/8250/8250_dw.c | 23 +++++++++++++++++------
>  1 file changed, 17 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/tty/serial/8250/8250_dw.c
> b/drivers/tty/serial/8250/8250_dw.c
> index d6934310..5ab9cfe 100644
> --- a/drivers/tty/serial/8250/8250_dw.c
> +++ b/drivers/tty/serial/8250/8250_dw.c
> @@ -297,12 +297,6 @@ static void dw8250_quirks(struct uart_port *p,
> struct dw8250_data *data)
>  			p->serial_in = dw8250_serial_in32be;
>  			p->serial_out = dw8250_serial_out32be;
>  		}
> -	} else if (has_acpi_companion(p->dev)) {
> -		p->iotype = UPIO_MEM32;
> -		p->regshift = 2;
> -		p->serial_in = dw8250_serial_in32;
> -		/* So far none of there implement the Busy
> Functionality */
> -		data->uart_16550_compatible = true;
>  	}
>  
>  	/* Platforms with iDMA */
> @@ -352,6 +346,19 @@ static void dw8250_setup_port(struct uart_port
> *p)
>  		up->capabilities |= UART_CAP_AFE;
>  }
>  
> +static struct property_entry dw8250_properties[] = {
> +	PROPERTY_ENTRY_U32("reg-io-width", 4),
> +	PROPERTY_ENTRY_U32("reg-shift", 2),
> +	PROPERTY_ENTRY_BOOL("snps,uart-16550-compatible"),
> +	{ },
> +};
> +
> +/* non 16550 compatible id list*/

non 16550 -> Non-16550
Space before */

> +static const struct acpi_device_id non_16550_ids[] = {
> +	{ "HISI0031", 0 },
> +	{ },
> +};
> +
>  static int dw8250_probe(struct platform_device *pdev)
>  {
>  	struct uart_8250_port uart = {};
> @@ -395,6 +402,9 @@ static int dw8250_probe(struct platform_device
> *pdev)
>  	if (!data)
>  		return -ENOMEM;
>  
> +	if (has_acpi_companion(dev) &&
> !acpi_match_device(non_16550_ids, dev))


> +		platform_device_add_properties(pdev,
> dw8250_properties);

What if we probe device which has already properties assigned?

Heikki, are you good with such trick?


> +
>  	data->dma.fn = dw8250_fallback_dma_filter;
>  	data->usr_reg = DW_UART_USR;
>  	p->private_data = data;
> @@ -619,6 +629,7 @@ static const struct acpi_device_id
> dw8250_acpi_match[] = {
>  	{ "APMC0D08", 0},
>  	{ "AMD0020", 0 },
>  	{ "AMDI0020", 0 },
> +	{ "HISI0031", 0 },
>  	{ },
>  };
>  MODULE_DEVICE_TABLE(acpi, dw8250_acpi_match);
Heikki Krogerus Aug. 22, 2016, 2:05 p.m. UTC | #3
Hi,

On Mon, Aug 22, 2016 at 02:24:14PM +0300, Andy Shevchenko wrote:
> On Fri, 2016-07-15 at 19:01 +0800, Kefeng Wang wrote:
> > Use built-in device properties to set device parameters for the
> > existing device probed by acpi.
> 
> acpi -> ACPI
> 
> > 
> > Add ACPI identifier for UART on Hisilicon Hip05 soc, be careful
> 
> soc -> SoC
> 
> > that it is not 16550 compatibal, so we need set "reg-io-width"
> 
> compatibal -> compatible
> 
> > and "reg-shift" by _DSD method in DSDT.
> > 
> > Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
> > ---
> >  drivers/tty/serial/8250/8250_dw.c | 23 +++++++++++++++++------
> >  1 file changed, 17 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/tty/serial/8250/8250_dw.c
> > b/drivers/tty/serial/8250/8250_dw.c
> > index d6934310..5ab9cfe 100644
> > --- a/drivers/tty/serial/8250/8250_dw.c
> > +++ b/drivers/tty/serial/8250/8250_dw.c
> > @@ -297,12 +297,6 @@ static void dw8250_quirks(struct uart_port *p,
> > struct dw8250_data *data)
> >  			p->serial_in = dw8250_serial_in32be;
> >  			p->serial_out = dw8250_serial_out32be;
> >  		}
> > -	} else if (has_acpi_companion(p->dev)) {
> > -		p->iotype = UPIO_MEM32;
> > -		p->regshift = 2;
> > -		p->serial_in = dw8250_serial_in32;
> > -		/* So far none of there implement the Busy
> > Functionality */
> > -		data->uart_16550_compatible = true;
> >  	}
> >  
> >  	/* Platforms with iDMA */
> > @@ -352,6 +346,19 @@ static void dw8250_setup_port(struct uart_port
> > *p)
> >  		up->capabilities |= UART_CAP_AFE;
> >  }
> >  
> > +static struct property_entry dw8250_properties[] = {
> > +	PROPERTY_ENTRY_U32("reg-io-width", 4),
> > +	PROPERTY_ENTRY_U32("reg-shift", 2),
> > +	PROPERTY_ENTRY_BOOL("snps,uart-16550-compatible"),
> > +	{ },
> > +};
> > +
> > +/* non 16550 compatible id list*/
> 
> non 16550 -> Non-16550
> Space before */
> 
> > +static const struct acpi_device_id non_16550_ids[] = {
> > +	{ "HISI0031", 0 },
> > +	{ },
> > +};
> > +
> >  static int dw8250_probe(struct platform_device *pdev)
> >  {
> >  	struct uart_8250_port uart = {};
> > @@ -395,6 +402,9 @@ static int dw8250_probe(struct platform_device
> > *pdev)
> >  	if (!data)
> >  		return -ENOMEM;
> >  
> > +	if (has_acpi_companion(dev) &&
> > !acpi_match_device(non_16550_ids, dev))
> 
> 
> > +		platform_device_add_properties(pdev,
> > dw8250_properties);
> 
> What if we probe device which has already properties assigned?
> 
> Heikki, are you good with such trick?

No this is not the way to do this. As we talked, we need to add the
properties in the mfd drivers when they exist and not in the driver
itself. Only if there is no mfd driver that creates the actual
platform device for dw8250 and when also the ACPI tables don't provide
the properties, we identify the board in dw8250_quirks and handle it
separately. Right now there is only one such board.

I'll prepare the patches for that right now. This series can then be
made on top of those.


Thanks,
Kefeng Wang Aug. 24, 2016, 8:22 a.m. UTC | #4
On 2016/8/22 22:05, Heikki Krogerus wrote:
> Hi,
> 
> On Mon, Aug 22, 2016 at 02:24:14PM +0300, Andy Shevchenko wrote:
>> On Fri, 2016-07-15 at 19:01 +0800, Kefeng Wang wrote:
>>> Use built-in device properties to set device parameters for the
>>> existing device probed by acpi.
>>
>> acpi -> ACPI
>>
>>>
>>> Add ACPI identifier for UART on Hisilicon Hip05 soc, be careful
>>
>> soc -> SoC
>>
>>> that it is not 16550 compatibal, so we need set "reg-io-width"
>>
>> compatibal -> compatible

Will fix.

>>
>>> and "reg-shift" by _DSD method in DSDT.
>>>
>>> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
>>> ---
>>>  drivers/tty/serial/8250/8250_dw.c | 23 +++++++++++++++++------
>>>  1 file changed, 17 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/tty/serial/8250/8250_dw.c
>>> b/drivers/tty/serial/8250/8250_dw.c
>>> index d6934310..5ab9cfe 100644
>>> --- a/drivers/tty/serial/8250/8250_dw.c
>>> +++ b/drivers/tty/serial/8250/8250_dw.c
>>> @@ -297,12 +297,6 @@ static void dw8250_quirks(struct uart_port *p,
>>> struct dw8250_data *data)
>>>  			p->serial_in = dw8250_serial_in32be;
>>>  			p->serial_out = dw8250_serial_out32be;
>>>  		}
>>> -	} else if (has_acpi_companion(p->dev)) {
>>> -		p->iotype = UPIO_MEM32;
>>> -		p->regshift = 2;
>>> -		p->serial_in = dw8250_serial_in32;
>>> -		/* So far none of there implement the Busy
>>> Functionality */
>>> -		data->uart_16550_compatible = true;
>>>  	}
>>>  
>>>  	/* Platforms with iDMA */
>>> @@ -352,6 +346,19 @@ static void dw8250_setup_port(struct uart_port
>>> *p)
>>>  		up->capabilities |= UART_CAP_AFE;
>>>  }
>>>  
>>> +static struct property_entry dw8250_properties[] = {
>>> +	PROPERTY_ENTRY_U32("reg-io-width", 4),
>>> +	PROPERTY_ENTRY_U32("reg-shift", 2),
>>> +	PROPERTY_ENTRY_BOOL("snps,uart-16550-compatible"),
>>> +	{ },
>>> +};
>>> +
>>> +/* non 16550 compatible id list*/
>>
>> non 16550 -> Non-16550
>> Space before */
>>
>>> +static const struct acpi_device_id non_16550_ids[] = {
>>> +	{ "HISI0031", 0 },
>>> +	{ },
>>> +};
>>> +
>>>  static int dw8250_probe(struct platform_device *pdev)
>>>  {
>>>  	struct uart_8250_port uart = {};
>>> @@ -395,6 +402,9 @@ static int dw8250_probe(struct platform_device
>>> *pdev)
>>>  	if (!data)
>>>  		return -ENOMEM;
>>>  
>>> +	if (has_acpi_companion(dev) &&
>>> !acpi_match_device(non_16550_ids, dev))
>>
>>
>>> +		platform_device_add_properties(pdev,
>>> dw8250_properties);
>>
>> What if we probe device which has already properties assigned?
>>
>> Heikki, are you good with such trick?
> 
> No this is not the way to do this. As we talked, we need to add the
> properties in the mfd drivers when they exist and not in the driver
> itself. Only if there is no mfd driver that creates the actual
> platform device for dw8250 and when also the ACPI tables don't provide
> the properties, we identify the board in dw8250_quirks and handle it
> separately. Right now there is only one such board.
> 
> I'll prepare the patches for that right now. This series can then be
> made on top of those.
> 

OK, will base on your patchset.

Thanks Andy and Heikki for your comments.

Kefeng


> 
> Thanks,
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index d6934310..5ab9cfe 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -297,12 +297,6 @@  static void dw8250_quirks(struct uart_port *p, struct dw8250_data *data)
 			p->serial_in = dw8250_serial_in32be;
 			p->serial_out = dw8250_serial_out32be;
 		}
-	} else if (has_acpi_companion(p->dev)) {
-		p->iotype = UPIO_MEM32;
-		p->regshift = 2;
-		p->serial_in = dw8250_serial_in32;
-		/* So far none of there implement the Busy Functionality */
-		data->uart_16550_compatible = true;
 	}
 
 	/* Platforms with iDMA */
@@ -352,6 +346,19 @@  static void dw8250_setup_port(struct uart_port *p)
 		up->capabilities |= UART_CAP_AFE;
 }
 
+static struct property_entry dw8250_properties[] = {
+	PROPERTY_ENTRY_U32("reg-io-width", 4),
+	PROPERTY_ENTRY_U32("reg-shift", 2),
+	PROPERTY_ENTRY_BOOL("snps,uart-16550-compatible"),
+	{ },
+};
+
+/* non 16550 compatible id list*/
+static const struct acpi_device_id non_16550_ids[] = {
+	{ "HISI0031", 0 },
+	{ },
+};
+
 static int dw8250_probe(struct platform_device *pdev)
 {
 	struct uart_8250_port uart = {};
@@ -395,6 +402,9 @@  static int dw8250_probe(struct platform_device *pdev)
 	if (!data)
 		return -ENOMEM;
 
+	if (has_acpi_companion(dev) && !acpi_match_device(non_16550_ids, dev))
+		platform_device_add_properties(pdev, dw8250_properties);
+
 	data->dma.fn = dw8250_fallback_dma_filter;
 	data->usr_reg = DW_UART_USR;
 	p->private_data = data;
@@ -619,6 +629,7 @@  static const struct acpi_device_id dw8250_acpi_match[] = {
 	{ "APMC0D08", 0},
 	{ "AMD0020", 0 },
 	{ "AMDI0020", 0 },
+	{ "HISI0031", 0 },
 	{ },
 };
 MODULE_DEVICE_TABLE(acpi, dw8250_acpi_match);