[2/8] powerpc/5200: Stop using device_type and port-number properties

From: Grant Likely <grant.likely@secretlab.ca>

There is no reason for the PSC UART driver or the Ethernet driver
to require a device_type property.  The compatible value is sufficient
to uniquely identify the device.  Remove it from the driver.

The whole 'port-number' scheme for assigning numbers to PSC uarts was
always rather half baked and just adds complexity.  Remove it from the
driver.  After this patch is applied, PSC UART numbers are simply
assigned from the order they are found in the device tree (just like
all the other devices).  Userspace can query sysfs to determine what
ttyPSC number is assigned to each PSC instance.

Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
CC: Wolfram Sang <w.sang@pengutronix.de>
---

 drivers/net/fec_mpc52xx.c     |    6 +++---
 drivers/serial/mpc52xx_uart.c |   38 ++++++++++----------------------------
 2 files changed, 13 insertions(+), 31 deletions(-)

Am Mittwoch, 21. Januar 2009 21:55 schrieb Grant Likely:
> [...]
> The whole 'port-number' scheme for assigning numbers to PSC uarts was
> always rather half baked and just adds complexity.  Remove it from the
> driver.  After this patch is applied, PSC UART numbers are simply
> assigned from the order they are found in the device tree (just like
> all the other devices).  Userspace can query sysfs to determine what
> ttyPSC number is assigned to each PSC instance.

Arrghh, the next time we have to touch every oftree of our supported boards to 
keep it work. Does it ever end?

Juergen

On Wed, Jan 21, 2009 at 2:13 PM, Juergen Beisert <jbe@pengutronix.de> wrote:
> Am Mittwoch, 21. Januar 2009 21:55 schrieb Grant Likely:
>> [...]
>> The whole 'port-number' scheme for assigning numbers to PSC uarts was
>> always rather half baked and just adds complexity.  Remove it from the
>> driver.  After this patch is applied, PSC UART numbers are simply
>> assigned from the order they are found in the device tree (just like
>> all the other devices).  Userspace can query sysfs to determine what
>> ttyPSC number is assigned to each PSC instance.
>
> Arrghh, the next time we have to touch every oftree of our supported boards to
> keep it work. Does it ever end?

This change will not break existing .dts files.  If it does, then it
is a bug.  Existing boards will still boot.

It may change the ttyPSC numbers that the kernel assigns at boot time.
 If so then, yes, it may require config changes in /etc.

The reason I want to do this is that trying to manipulate the
assignments using port-number resulted in a lot of confusion and it
had the driver trying to do policy stuff (especially considering
port-number is completely arbitrary and doesn't even have any relation
to the physical PSC numbering from the 5200 user manual).  I want to
get away from that entirely and go with a strict probe order
enumeration of PSC ports since there are more reliable methods to map
physical ports to ttyPSC numbers (assuming it really needs to be
detected at runtime instead of statically configured).

g.

On Wed, Jan 21, 2009 at 01:55:13PM -0700, Grant Likely wrote:
> From: Grant Likely <grant.likely@secretlab.ca>
> 
> There is no reason for the PSC UART driver or the Ethernet driver
> to require a device_type property.  The compatible value is sufficient
> to uniquely identify the device.  Remove it from the driver.
> 
> The whole 'port-number' scheme for assigning numbers to PSC uarts was
> always rather half baked and just adds complexity.  Remove it from the
> driver.  After this patch is applied, PSC UART numbers are simply
> assigned from the order they are found in the device tree (just like
> all the other devices).  Userspace can query sysfs to determine what
> ttyPSC number is assigned to each PSC instance.
> 
> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
> CC: Wolfram Sang <w.sang@pengutronix.de>

I like it. One more optimization below, but it's also good for now...

Reviewed-by: Wolfram Sang <w.sang@pengutronix.de>

> ---
> 
>  drivers/net/fec_mpc52xx.c     |    6 +++---
>  drivers/serial/mpc52xx_uart.c |   38 ++++++++++----------------------------
>  2 files changed, 13 insertions(+), 31 deletions(-)
> 
> 
> diff --git a/drivers/net/fec_mpc52xx.c b/drivers/net/fec_mpc52xx.c
> index cd8e98b..049b0a7 100644
> --- a/drivers/net/fec_mpc52xx.c
> +++ b/drivers/net/fec_mpc52xx.c
> @@ -1123,9 +1123,9 @@ static int mpc52xx_fec_of_resume(struct of_device *op)
>  #endif
>  
>  static struct of_device_id mpc52xx_fec_match[] = {
> -	{ .type = "network", .compatible = "fsl,mpc5200b-fec", },
> -	{ .type = "network", .compatible = "fsl,mpc5200-fec", },
> -	{ .type = "network", .compatible = "mpc5200-fec", },
> +	{ .compatible = "fsl,mpc5200b-fec", },
> +	{ .compatible = "fsl,mpc5200-fec", },
> +	{ .compatible = "mpc5200-fec", },
>  	{ }
>  };
>  
> diff --git a/drivers/serial/mpc52xx_uart.c b/drivers/serial/mpc52xx_uart.c
> index 0c3a2ab..d73d7da 100644
> --- a/drivers/serial/mpc52xx_uart.c
> +++ b/drivers/serial/mpc52xx_uart.c
> @@ -50,8 +50,8 @@
>  /* OF Platform device Usage :
>   *
>   * This driver is only used for PSCs configured in uart mode.  The device
> - * tree will have a node for each PSC in uart mode w/ device_type = "serial"
> - * and "mpc52xx-psc-uart" in the compatible string
> + * tree will have a node for each PSC with "mpc52xx-psc-uart" in the compatible
> + * list.
>   *
>   * By default, PSC devices are enumerated in the order they are found.  However
>   * a particular PSC number can be forces by adding 'device_no = <port#>'
> @@ -1212,30 +1212,18 @@ mpc52xx_uart_of_resume(struct of_device *op)
>  #endif
>  
>  static void
> -mpc52xx_uart_of_assign(struct device_node *np, int idx)
> +mpc52xx_uart_of_assign(struct device_node *np)
>  {
> -	int free_idx = -1;
>  	int i;
>  
> -	/* Find the first free node */
> +	/* Find the first free PSC number */
>  	for (i = 0; i < MPC52xx_PSC_MAXNUM; i++) {
>  		if (mpc52xx_uart_nodes[i] == NULL) {
> -			free_idx = i;
> -			break;
> +			of_node_get(np);
> +			mpc52xx_uart_nodes[i] = np;
> +			return;
>  		}
>  	}
> -
> -	if ((idx < 0) || (idx >= MPC52xx_PSC_MAXNUM))
> -		idx = free_idx;
> -
> -	if (idx < 0)
> -		return; /* No free slot; abort */
> -
> -	of_node_get(np);
> -	/* If the slot is already occupied, then swap slots */
> -	if (mpc52xx_uart_nodes[idx] && (free_idx != -1))
> -		mpc52xx_uart_nodes[free_idx] = mpc52xx_uart_nodes[idx];
> -	mpc52xx_uart_nodes[idx] = np;
>  }
>  
>  static void
> @@ -1243,23 +1231,17 @@ mpc52xx_uart_of_enumerate(void)
>  {
>  	static int enum_done;
>  	struct device_node *np;
> -	const unsigned int *devno;
>  	const struct  of_device_id *match;
>  	int i;
>  
>  	if (enum_done)
>  		return;
>  
> -	for_each_node_by_type(np, "serial") {
> +	/* Assign index to each PSC in device tree */
> +	for_each_matching_node(np, mpc52xx_uart_of_match) {
>  		match = of_match_node(mpc52xx_uart_of_match, np);
> -		if (!match)
> -			continue;
> -
>  		psc_ops = match->data;
> -
> -		/* Is a particular device number requested? */
> -		devno = of_get_property(np, "port-number", NULL);
> -		mpc52xx_uart_of_assign(np, devno ? *devno : -1);
> +		mpc52xx_uart_of_assign(np);
>  	}
>  
>  	enum_done = 1;
> 

You could drop the for-loop which follows here and put its dev_dbg into
uart_of_assign when a node was found. Would also get rid of 'i' here.

Regards,

   Wolfram