Re: [PATCH] using mii-bitbang on different processor ports

[Scott Wood <scottwood@freescale.com>]

On Tue, Oct 30, 2007 at 05:09:19PM +0100, Sergej Stepanov wrote:
> The patch makes possible to have mdio and mdc pins on different physical ports
> also for CONFIG_PPC_CPM_NEW_BINDING.
> To setup it in the device tree:
> reg = <10d40 14 10d60 14>; // mdc-offset: 0x10d40, mdio-offset: 0x10d60
> or
> reg = <10d40 14>; // mdc and mdio have the same offset 0x10d40
> The approach was taken from older version.

There are some formatting issues in fs_mii_bitbang_init(), but otherwise it
looks good.  It'll need to be sent to Jeff Garzik and the netdev list, not
just linuxppc-dev, though.

Also, please update Documentation/powerpc/booting-without-of.txt (probably
in a separate patch, since that one would go through Paul).

> @@ -142,13 +146,27 @@ static int __devinit fs_mii_bitbang_init(struct mii_bus *bus,
>  		return -ENODEV;
>  	mdc_pin = *data;
>  
> -	bitbang->dir = ioremap(res.start, res.end - res.start + 1);
> -	if (!bitbang->dir)
> +	bitbang->mdc.dir = ioremap(res[0].start, res[0].end - res[0].start + 1);
> +	if (!bitbang->mdc.dir)
>  		return -ENOMEM;
>  
> -	bitbang->dat = bitbang->dir + 4;
> -	bitbang->mdio_msk = 1 << (31 - mdio_pin);
> -	bitbang->mdc_msk = 1 << (31 - mdc_pin);
> +	bitbang->mdc.dat = bitbang->mdc.dir + 4;
> +	if( !of_address_to_resource(np, 1, &res[1]))

Space before the '(', not after.  A newline after the previous line would be
nice, too.

> +	{

Brace at the end of the previous line.

> +		bitbang->mdio.dir = ioremap(res[1].start, res[1].end - res[1].start + 1);
> +		if (!bitbang->mdio.dir)
> +		{

Likewise.

You could just use of_iomap() for the second one, since we don't need
the physical address for bus->id.

> +			iounmap(bitbang->mdc.dir);
> +			return -ENOMEM;

Please use the goto-style error handling that's used elsewhere in the
function.

> +		}
> +		bitbang->mdio.dat = bitbang->mdio.dir + 4;
> +	}
> +	else{

} else {

>  out_unmap_regs:
> -	iounmap(bitbang->dir);
> +	if ( bitbang->mdio.dir != bitbang->mdc.dir)
> +		iounmap(bitbang->mdio.dir);
> +	iounmap(bitbang->mdc.dir);
>  out_free_bus:
>  	kfree(new_bus);
>  out_free_priv:
> @@ -238,7 +258,9 @@ static int fs_enet_mdio_remove(struct of_device *ofdev)
>  	free_mdio_bitbang(bus);
>  	dev_set_drvdata(&ofdev->dev, NULL);
>  	kfree(bus->irq);
> -	iounmap(bitbang->dir);
> +	if ( bitbang->mdio.dir != bitbang->mdc.dir)
> +		iounmap(bitbang->mdio.dir);
> +	iounmap(bitbang->mdc.dir);
>  	kfree(bitbang);
>  	kfree(bus);

"if (bitbang", not "if ( bitbang".

-Scott