[PATCH 1/2] ne: DeviceTree support.

[PATCH 1/2] ne: DeviceTree support.

[Yoshinori Sato]

Add basic device tree support.

Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>
---
 Documentation/devicetree/bindings/net/ne2000.txt | 17 +++++++++++++++++
 drivers/net/ethernet/8390/ne.c                   | 20 +++++++++++++++++++-
 2 files changed, 36 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/net/ne2000.txt

diff --git a/Documentation/devicetree/bindings/net/ne2000.txt b/Documentation/devicetree/bindings/net/ne2000.txt
new file mode 100644
index 0000000..8b0dfbf
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/ne2000.txt
@@ -0,0 +1,17 @@
+NE2000 compatible network controller
+
+Required properties:
+- compatible: "national,ne2000"
+- reg: base address and length of NE2000.
+- interrupts: interrupt specifier for the sole interrupt.
+- national,dcr: DCR setting value.
+
+Example
+
+	ne2000: ethernet@200000 {
+		compatible = "national,ne2000";
+		reg = <0x200000 32>;
+		interrupts = <17 0>;
+		national,dcr = <0x48>;
+	};
+
diff --git a/drivers/net/ethernet/8390/ne.c b/drivers/net/ethernet/8390/ne.c
index c063b41..a9dde5b 100644
--- a/drivers/net/ethernet/8390/ne.c
+++ b/drivers/net/ethernet/8390/ne.c
@@ -52,6 +52,7 @@ static const char version2[] =
 #include <linux/etherdevice.h>
 #include <linux/jiffies.h>
 #include <linux/platform_device.h>
+#include <linux/of.h>
 
 #include <asm/io.h>
 
@@ -72,6 +73,7 @@ static int io[MAX_NE_CARDS];
 static int irq[MAX_NE_CARDS];
 static int bad[MAX_NE_CARDS];
 static u32 ne_msg_enable;
+static unsigned int of_dcr_val;
 
 #ifdef MODULE
 module_param_array(io, int, NULL, 0);
@@ -171,6 +173,8 @@ bad_clone_list[] __initdata = {
 #  define DCR_VAL 0x48		/* 8-bit mode */
 #elif defined(CONFIG_ATARI)	/* 8-bit mode on Atari, normal on Q40 */
 #  define DCR_VAL (MACH_IS_ATARI ? 0x48 : 0x49)
+#elif defined(CONFIG_OF_NET)
+#  define DCR_VAL of_dcr_val
 #else
 #  define DCR_VAL 0x49
 #endif
@@ -304,7 +308,8 @@ static int __init ne_probe1(struct net_device *dev, unsigned long ioaddr)
 	struct ei_device *ei_local = netdev_priv(dev);
 
 	if (!request_region(ioaddr, NE_IO_EXTENT, DRV_NAME))
-		return -EBUSY;
+		if (!request_mem_region(ioaddr, NE_IO_EXTENT, DRV_NAME))
+			return -EBUSY;
 
 	reg0 = inb_p(ioaddr);
 	if (reg0 == 0xFF) {
@@ -808,11 +813,18 @@ static int __init ne_drv_probe(struct platform_device *pdev)
 	if (!dev)
 		return -ENOMEM;
 
+	if (dev_of_node(&pdev->dev))
+		of_property_read_u32(dev_of_node(&pdev->dev),
+				     "national,dcr", &of_dcr_val);
+
 	/* ne.c doesn't populate resources in platform_device, but
 	 * rbtx4927_ne_init and rbtx4938_ne_init do register devices
 	 * with resources.
 	 */
 	res = platform_get_resource(pdev, IORESOURCE_IO, 0);
+	if (!res)
+		res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+
 	if (res) {
 		dev->base_addr = res->start;
 		dev->irq = platform_get_irq(pdev, 0);
@@ -914,12 +926,18 @@ static int ne_drv_resume(struct platform_device *pdev)
 #define ne_drv_resume NULL
 #endif
 
+static const struct of_device_id ne2000_of_table[] __maybe_unused = {
+	{ .compatible = "national,ne2000" },
+	{ }
+};
+
 static struct platform_driver ne_driver = {
 	.remove		= ne_drv_remove,
 	.suspend	= ne_drv_suspend,
 	.resume		= ne_drv_resume,
 	.driver		= {
 		.name	= DRV_NAME,
+		.of_match_table = of_match_ptr(ne2000_of_table),
 	},
 };
 
-- 
2.6.1

[PATCH 2/2] ne: Add h8300 support.

[Yoshinori Sato]

Add H8/300 platform support.

Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>
---
 drivers/net/ethernet/8390/Kconfig | 2 +-
 drivers/net/ethernet/8390/ne.c    | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/8390/Kconfig b/drivers/net/ethernet/8390/Kconfig
index 29c3075..7530bd9 100644
--- a/drivers/net/ethernet/8390/Kconfig
+++ b/drivers/net/ethernet/8390/Kconfig
@@ -88,7 +88,7 @@ config MCF8390
 config NE2000
 	tristate "NE2000/NE1000 support"
 	depends on (ISA || (Q40 && m) || M32R || MACH_TX49XX || \
-		    ATARI_ETHERNEC)
+		    ATARI_ETHERNEC || H8300)
 	select CRC32
 	---help---
 	  If you have a network (Ethernet) card of this type, say Y here.
diff --git a/drivers/net/ethernet/8390/ne.c b/drivers/net/ethernet/8390/ne.c
index a9dde5b..e0ed765 100644
--- a/drivers/net/ethernet/8390/ne.c
+++ b/drivers/net/ethernet/8390/ne.c
@@ -427,7 +427,8 @@ static int __init ne_probe1(struct net_device *dev, unsigned long ioaddr)
 		stop_page  = NE1SM_STOP_PG;
 	}
 
-#if  defined(CONFIG_PLAT_MAPPI) || defined(CONFIG_PLAT_OAKS32R)
+#if  defined(CONFIG_PLAT_MAPPI) || defined(CONFIG_PLAT_OAKS32R) || \
+	defined(CONFIG_H8300)
 	neX000 = ((SA_prom[14] == 0x57  &&  SA_prom[15] == 0x57)
 		|| (SA_prom[14] == 0x42 && SA_prom[15] == 0x42));
 #else
-- 
2.6.1

Re: [PATCH 1/2] ne: DeviceTree support.

[Andrew Lunn]

On Sat, Jan 16, 2016 at 01:19:45AM +0900, Yoshinori Sato wrote:
> Add basic device tree support.
> 
> Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>
> ---
>  Documentation/devicetree/bindings/net/ne2000.txt | 17 +++++++++++++++++
>  drivers/net/ethernet/8390/ne.c                   | 20 +++++++++++++++++++-
>  2 files changed, 36 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/devicetree/bindings/net/ne2000.txt
> 
> diff --git a/Documentation/devicetree/bindings/net/ne2000.txt b/Documentation/devicetree/bindings/net/ne2000.txt
> new file mode 100644
> index 0000000..8b0dfbf
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/ne2000.txt
> @@ -0,0 +1,17 @@
> +NE2000 compatible network controller
> +
> +Required properties:
> +- compatible: "national,ne2000"
> +- reg: base address and length of NE2000.
> +- interrupts: interrupt specifier for the sole interrupt.
> +- national,dcr: DCR setting value.

You say here that national,dcr is required, yet the code to read it is
not returning an error if it is missing.

Also, what is DCR? 

> +
> +Example
> +
> +	ne2000: ethernet@200000 {
> +		compatible = "national,ne2000";
> +		reg = <0x200000 32>;
> +		interrupts = <17 0>;
> +		national,dcr = <0x48>;
> +	};
> +
> diff --git a/drivers/net/ethernet/8390/ne.c b/drivers/net/ethernet/8390/ne.c
> index c063b41..a9dde5b 100644
> --- a/drivers/net/ethernet/8390/ne.c
> +++ b/drivers/net/ethernet/8390/ne.c
> @@ -52,6 +52,7 @@ static const char version2[] =
>  #include <linux/etherdevice.h>
>  #include <linux/jiffies.h>
>  #include <linux/platform_device.h>
> +#include <linux/of.h>
>  
>  #include <asm/io.h>
>  
> @@ -72,6 +73,7 @@ static int io[MAX_NE_CARDS];
>  static int irq[MAX_NE_CARDS];
>  static int bad[MAX_NE_CARDS];
>  static u32 ne_msg_enable;
> +static unsigned int of_dcr_val;

So there is a single DCR value, for all instances of the device?
The last device to probe wins?
  
  Andrew

Re: [PATCH 1/2] ne: DeviceTree support.

[Yoshinori Sato]

On Sun, 17 Jan 2016 02:22:26 +0900,
Andrew Lunn wrote:
> 
> On Sat, Jan 16, 2016 at 01:19:45AM +0900, Yoshinori Sato wrote:
> > Add basic device tree support.
> > 
> > Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>
> > ---
> >  Documentation/devicetree/bindings/net/ne2000.txt | 17 +++++++++++++++++
> >  drivers/net/ethernet/8390/ne.c                   | 20 +++++++++++++++++++-
> >  2 files changed, 36 insertions(+), 1 deletion(-)
> >  create mode 100644 Documentation/devicetree/bindings/net/ne2000.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/net/ne2000.txt b/Documentation/devicetree/bindings/net/ne2000.txt
> > new file mode 100644
> > index 0000000..8b0dfbf
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/ne2000.txt
> > @@ -0,0 +1,17 @@
> > +NE2000 compatible network controller
> > +
> > +Required properties:
> > +- compatible: "national,ne2000"
> > +- reg: base address and length of NE2000.
> > +- interrupts: interrupt specifier for the sole interrupt.
> > +- national,dcr: DCR setting value.
> 
> You say here that national,dcr is required, yet the code to read it is
> not returning an error if it is missing.

Yes. This value required. Missing error check.

> Also, what is DCR?

This is chip configuration.
It value depend on target design.

> 
> > +
> > +Example
> > +
> > +	ne2000: ethernet@200000 {
> > +		compatible = "national,ne2000";
> > +		reg = <0x200000 32>;
> > +		interrupts = <17 0>;
> > +		national,dcr = <0x48>;
> > +	};
> > +
> > diff --git a/drivers/net/ethernet/8390/ne.c b/drivers/net/ethernet/8390/ne.c
> > index c063b41..a9dde5b 100644
> > --- a/drivers/net/ethernet/8390/ne.c
> > +++ b/drivers/net/ethernet/8390/ne.c
> > @@ -52,6 +52,7 @@ static const char version2[] =
> >  #include <linux/etherdevice.h>
> >  #include <linux/jiffies.h>
> >  #include <linux/platform_device.h>
> > +#include <linux/of.h>
> >  
> >  #include <asm/io.h>
> >  
> > @@ -72,6 +73,7 @@ static int io[MAX_NE_CARDS];
> >  static int irq[MAX_NE_CARDS];
> >  static int bad[MAX_NE_CARDS];
> >  static u32 ne_msg_enable;
> > +static unsigned int of_dcr_val;
> 
> So there is a single DCR value, for all instances of the device?
> The last device to probe wins?

Yes. It'll be usually the same value by all devices.

>   
>   Andrew

-- 
Yoshinori Sato
<ysato@users.sourceforge.jp>

[PATCH v2 1/2] ne: DeviceTree support.

[Yoshinori Sato]

Add basic device tree support.
Changes for v2
- Add "national,dcr" property read check.

Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>
---
 Documentation/devicetree/bindings/net/ne2000.txt | 17 +++++++++++
 drivers/net/ethernet/8390/ne.c                   | 36 +++++++++++++++++++-----
 2 files changed, 46 insertions(+), 7 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/ne2000.txt

diff --git a/Documentation/devicetree/bindings/net/ne2000.txt b/Documentation/devicetree/bindings/net/ne2000.txt
new file mode 100644
index 0000000..8b0dfbf
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/ne2000.txt
@@ -0,0 +1,17 @@
+NE2000 compatible network controller
+
+Required properties:
+- compatible: "national,ne2000"
+- reg: base address and length of NE2000.
+- interrupts: interrupt specifier for the sole interrupt.
+- national,dcr: DP8390 DCR setting value.
+
+Example
+
+	ne2000: ethernet@200000 {
+		compatible = "national,ne2000";
+		reg = <0x200000 32>;
+		interrupts = <17 0>;
+		national,dcr = <0x48>;
+	};
+
diff --git a/drivers/net/ethernet/8390/ne.c b/drivers/net/ethernet/8390/ne.c
index c063b41..f1c21c6 100644
--- a/drivers/net/ethernet/8390/ne.c
+++ b/drivers/net/ethernet/8390/ne.c
@@ -52,6 +52,7 @@ static const char version2[] =
 #include <linux/etherdevice.h>
 #include <linux/jiffies.h>
 #include <linux/platform_device.h>
+#include <linux/of.h>
 
 #include <asm/io.h>
 
@@ -72,6 +73,7 @@ static int io[MAX_NE_CARDS];
 static int irq[MAX_NE_CARDS];
 static int bad[MAX_NE_CARDS];
 static u32 ne_msg_enable;
+static unsigned int of_dcr_val;
 
 #ifdef MODULE
 module_param_array(io, int, NULL, 0);
@@ -171,6 +173,8 @@ bad_clone_list[] __initdata = {
 #  define DCR_VAL 0x48		/* 8-bit mode */
 #elif defined(CONFIG_ATARI)	/* 8-bit mode on Atari, normal on Q40 */
 #  define DCR_VAL (MACH_IS_ATARI ? 0x48 : 0x49)
+#elif defined(CONFIG_OF_NET)
+#  define DCR_VAL of_dcr_val
 #else
 #  define DCR_VAL 0x49
 #endif
@@ -304,7 +308,8 @@ static int __init ne_probe1(struct net_device *dev, unsigned long ioaddr)
 	struct ei_device *ei_local = netdev_priv(dev);
 
 	if (!request_region(ioaddr, NE_IO_EXTENT, DRV_NAME))
-		return -EBUSY;
+		if (!request_mem_region(ioaddr, NE_IO_EXTENT, DRV_NAME))
+			return -EBUSY;
 
 	reg0 = inb_p(ioaddr);
 	if (reg0 == 0xFF) {
@@ -808,18 +813,28 @@ static int __init ne_drv_probe(struct platform_device *pdev)
 	if (!dev)
 		return -ENOMEM;
 
+	if (dev_of_node(&pdev->dev)) {
+		err = of_property_read_u32(dev_of_node(&pdev->dev),
+					   "national,dcr", &of_dcr_val);
+		if (err)
+			goto fail;
+	}
+
 	/* ne.c doesn't populate resources in platform_device, but
 	 * rbtx4927_ne_init and rbtx4938_ne_init do register devices
 	 * with resources.
 	 */
 	res = platform_get_resource(pdev, IORESOURCE_IO, 0);
+	if (!res)
+		res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+
 	if (res) {
 		dev->base_addr = res->start;
 		dev->irq = platform_get_irq(pdev, 0);
 	} else {
 		if (this_dev < 0 || this_dev >= MAX_NE_CARDS) {
-			free_netdev(dev);
-			return -EINVAL;
+			err = -EINVAL;
+			goto fail;
 		}
 		dev->base_addr = io[this_dev];
 		dev->irq = irq[this_dev];
@@ -827,10 +842,8 @@ static int __init ne_drv_probe(struct platform_device *pdev)
 	}
 	SET_NETDEV_DEV(dev, &pdev->dev);
 	err = do_ne_probe(dev);
-	if (err) {
-		free_netdev(dev);
-		return err;
-	}
+	if (err)
+		goto fail;
 	platform_set_drvdata(pdev, dev);
 
 	/* Update with any values found by probing, don't update if
@@ -841,6 +854,9 @@ static int __init ne_drv_probe(struct platform_device *pdev)
 		irq[this_dev] = dev->irq;
 	}
 	return 0;
+fail:
+	free_netdev(dev);
+	return err;
 }
 
 static int ne_drv_remove(struct platform_device *pdev)
@@ -914,12 +930,18 @@ static int ne_drv_resume(struct platform_device *pdev)
 #define ne_drv_resume NULL
 #endif
 
+static const struct of_device_id ne2000_of_table[] __maybe_unused = {
+	{ .compatible = "national,ne2000" },
+	{ }
+};
+
 static struct platform_driver ne_driver = {
 	.remove		= ne_drv_remove,
 	.suspend	= ne_drv_suspend,
 	.resume		= ne_drv_resume,
 	.driver		= {
 		.name	= DRV_NAME,
+		.of_match_table = of_match_ptr(ne2000_of_table),
 	},
 };
 
-- 
2.6.1

[PATCH v2 2/2] ne: Add h8300 support.

[Yoshinori Sato]

Add H8/300 platform support.

Changes fot v2
- None

Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>
---
 drivers/net/ethernet/8390/Kconfig | 2 +-
 drivers/net/ethernet/8390/ne.c    | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/8390/Kconfig b/drivers/net/ethernet/8390/Kconfig
index 29c3075..7530bd9 100644
--- a/drivers/net/ethernet/8390/Kconfig
+++ b/drivers/net/ethernet/8390/Kconfig
@@ -88,7 +88,7 @@ config MCF8390
 config NE2000
 	tristate "NE2000/NE1000 support"
 	depends on (ISA || (Q40 && m) || M32R || MACH_TX49XX || \
-		    ATARI_ETHERNEC)
+		    ATARI_ETHERNEC || H8300)
 	select CRC32
 	---help---
 	  If you have a network (Ethernet) card of this type, say Y here.
diff --git a/drivers/net/ethernet/8390/ne.c b/drivers/net/ethernet/8390/ne.c
index f1c21c6..b7b8b8b 100644
--- a/drivers/net/ethernet/8390/ne.c
+++ b/drivers/net/ethernet/8390/ne.c
@@ -427,7 +427,8 @@ static int __init ne_probe1(struct net_device *dev, unsigned long ioaddr)
 		stop_page  = NE1SM_STOP_PG;
 	}
 
-#if  defined(CONFIG_PLAT_MAPPI) || defined(CONFIG_PLAT_OAKS32R)
+#if  defined(CONFIG_PLAT_MAPPI) || defined(CONFIG_PLAT_OAKS32R) || \
+	defined(CONFIG_H8300)
 	neX000 = ((SA_prom[14] == 0x57  &&  SA_prom[15] == 0x57)
 		|| (SA_prom[14] == 0x42 && SA_prom[15] == 0x42));
 #else
-- 
2.6.1

Re: [PATCH 1/2] ne: DeviceTree support.

[Andrew Lunn]

On Mon, Jan 18, 2016 at 04:09:40PM +0900, Yoshinori Sato wrote:
> On Sun, 17 Jan 2016 02:22:26 +0900,
> Andrew Lunn wrote:
> > 
> > On Sat, Jan 16, 2016 at 01:19:45AM +0900, Yoshinori Sato wrote:
> > > Add basic device tree support.
> > > 
> > > Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>
> > > ---
> > >  Documentation/devicetree/bindings/net/ne2000.txt | 17 +++++++++++++++++
> > >  drivers/net/ethernet/8390/ne.c                   | 20 +++++++++++++++++++-
> > >  2 files changed, 36 insertions(+), 1 deletion(-)
> > >  create mode 100644 Documentation/devicetree/bindings/net/ne2000.txt
> > > 
> > > diff --git a/Documentation/devicetree/bindings/net/ne2000.txt b/Documentation/devicetree/bindings/net/ne2000.txt
> > > new file mode 100644
> > > index 0000000..8b0dfbf
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/net/ne2000.txt
> > > @@ -0,0 +1,17 @@
> > > +NE2000 compatible network controller
> > > +
> > > +Required properties:
> > > +- compatible: "national,ne2000"
> > > +- reg: base address and length of NE2000.
> > > +- interrupts: interrupt specifier for the sole interrupt.
> > > +- national,dcr: DCR setting value.
> > 
> > You say here that national,dcr is required, yet the code to read it is
> > not returning an error if it is missing.
> 
> Yes. This value required. Missing error check.
> 
> > Also, what is DCR?
> 
> This is chip configuration.
> It value depend on target design.

It needs to be described in detail what it is. Device tree bindings
generally don't list values to be poked into registers. They describe
something, and from that, the value to be poked into a register is
derived.

> > So there is a single DCR value, for all instances of the device?
> > The last device to probe wins?
> 
> Yes. It'll be usually the same value by all devices.

There is no 'usually' about it. You implementation forces them all to
be the same. You need to add error checking. If different DT instances
have different values, you need to issue an error and fail one or more
probes.

You also need to document this in the device tree binding.

	Andrew

Re: [PATCH v2 1/2] ne: DeviceTree support.

[David Miller]

From: Yoshinori Sato <ysato@users.sourceforge.jp>
Date: Mon, 18 Jan 2016 22:32:36 +0900

> Add basic device tree support.
> Changes for v2
> - Add "national,dcr" property read check.
 ...
> +		national,dcr = <0x48>;

As has been explained, this national.dcr property is inappropriate.

Re: [PATCH 1/2] ne: DeviceTree support.

[David Miller]

From: Andrew Lunn <andrew@lunn.ch>
Date: Mon, 18 Jan 2016 16:08:34 +0100

> On Mon, Jan 18, 2016 at 04:09:40PM +0900, Yoshinori Sato wrote:
>> On Sun, 17 Jan 2016 02:22:26 +0900,
>> Andrew Lunn wrote:
>> > 
>> > On Sat, Jan 16, 2016 at 01:19:45AM +0900, Yoshinori Sato wrote:
>> > > Add basic device tree support.
>> > > 
>> > > Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>
>> > > ---
>> > >  Documentation/devicetree/bindings/net/ne2000.txt | 17 +++++++++++++++++
>> > >  drivers/net/ethernet/8390/ne.c                   | 20 +++++++++++++++++++-
>> > >  2 files changed, 36 insertions(+), 1 deletion(-)
>> > >  create mode 100644 Documentation/devicetree/bindings/net/ne2000.txt
>> > > 
>> > > diff --git a/Documentation/devicetree/bindings/net/ne2000.txt b/Documentation/devicetree/bindings/net/ne2000.txt
>> > > new file mode 100644
>> > > index 0000000..8b0dfbf
>> > > --- /dev/null
>> > > +++ b/Documentation/devicetree/bindings/net/ne2000.txt
>> > > @@ -0,0 +1,17 @@
>> > > +NE2000 compatible network controller
>> > > +
>> > > +Required properties:
>> > > +- compatible: "national,ne2000"
>> > > +- reg: base address and length of NE2000.
>> > > +- interrupts: interrupt specifier for the sole interrupt.
>> > > +- national,dcr: DCR setting value.
>> > 
>> > You say here that national,dcr is required, yet the code to read it is
>> > not returning an error if it is missing.
>> 
>> Yes. This value required. Missing error check.
>> 
>> > Also, what is DCR?
>> 
>> This is chip configuration.
>> It value depend on target design.
> 
> It needs to be described in detail what it is. Device tree bindings
> generally don't list values to be poked into registers. They describe
> something, and from that, the value to be poked into a register is
> derived.

Agreed.