Re: [v4,1/9] net-next: phy: new Asix Electronics PHY driver

2019-01-20 Thread Andrew Lunn 
On Fri, Jan 18, 2019 at 11:22:39AM +0100, Thomas Gleixner wrote:
> Michael,
> 
> On Thu, 19 Apr 2018, Michael Schmitz wrote:
> 
> > --- /dev/null
> > +++ b/drivers/net/phy/asix.c
> > @@ -0,0 +1,63 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/* Driver for Asix PHYs
> > + *
> > + * Author: Michael Schmitz 
> > + *
> > + * This program is free software; you can redistribute  it and/or modify it
> > + * under  the terms of  the GNU General  Public License as published by the
> > + * Free Software Foundation;  either version 2 of the  License, or (at your
> > + * option) any later version.
> > + *
> > + */
> 
> This license information is broken. The SPDX license identifier and the
> boiler plate text are contradicting. The SPDX id is GPL v2 only and the
> boiler plate says v2 or later.

Hi Thomas

Please see:

https://www.spinics.net/lists/netdev/msg544312.html

The first two patches are simple SPDX converstions. Then it gets
interesting trying to sort out license inconsistencies.

Andrew

Re: [PATCH v4 1/9] net-next: phy: new Asix Electronics PHY driver

2018-04-19 Thread Andrew Lunn 
On Thu, Apr 19, 2018 at 02:05:18PM +1200, Michael Schmitz wrote:
> The Asix Electronics PHY found on the X-Surf 100 Amiga Zorro network
> card by Individual Computers is buggy, and needs the reset bit toggled
> as workaround to make a PHY soft reset succeed.
> 
> Add workaround driver just for this special case.
> 
> Suggested in xsurf100 patch series review by Andrew Lunn <and...@lunn.ch>
> 
> Signed-off-by: Michael Schmitz <schmitz...@gmail.com>

Reviewed-by: Andrew Lunn <and...@lunn.ch>

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

Re: [PATCH v3 00/10] New network driver for Amiga X-Surf 100 (m68k)

2018-04-18 Thread Andrew Lunn 
On Wed, Apr 18, 2018 at 05:10:45PM +1200, Michael Schmitz wrote:
> All,
> 
> just noticed belatedly that the Makefile hunk of patch 9 does no
> longer apply cleanly in 4.17-rc1, sorry. My series was based on 4.16.
> I'll resend that one, OK?

Hi Michael

You should be based on DaveM net-next tree:

git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git

Please also have "net-next" in the patch subject. See
Documentation/networking/netdev-FAQ.txt

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

Re: [PATCH v3 1/9] net: phy: new Asix Electronics PHY driver

2018-04-18 Thread Andrew Lunn 
> +
> +/**
> + * asix_soft_reset - software reset the PHY via BMCR_RESET bit
> + * @phydev: target phy_device struct
> + *
> + * Description: Perform a software PHY reset using the standard
> + * BMCR_RESET bit and poll for the reset bit to be cleared.
> + * Toggle BMCR_RESET bit off to accomodate broken PHY implementations
> + * such as used on the Individual Computers' X-Surf 100 Zorro card.
> + *
> + * Returns: 0 on success, < 0 on failure
> + */
> +static int asix_soft_reset(struct phy_device *phydev)
> +{
> + int ret;
> +
> + /* Asix PHY won't reset unless reset bit toggles */
> + ret = phy_write(phydev, MII_BMCR, 0);
> + if (ret < 0)
> + return ret;
> +
> + phy_write(phydev, MII_BMCR, BMCR_RESET);
> +
> + return phy_poll_reset(phydev);
> +}

Why not simply:

static int asix_soft_reset(struct phy_device *phydev)
{
int ret;

/* Asix PHY won't reset unless reset bit toggles */
ret = phy_write(phydev, MII_BMCR, 0);
if (ret < 0)
return ret;

return genphy_soft_reset(phydev);
}

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

Re: [PATCH 04/10] net: ax88796: Add block_input/output hooks to ax_plat_data

2018-04-17 Thread Andrew Lunn 
On Wed, Apr 18, 2018 at 12:53:21PM +1200, Michael Schmitz wrote:
> I think this is a false positive - we're encouraged to provide the
> full parameter list for functions, so the sreuct sk_buff* can't be
> avoided.

Hi Michael

How is  being included?

You probably want to build using the .config file and see.

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

Re: [PATCH v2 3/8] net: ax88796: Do not free IRQ in ax_remove() (already freed in ax_close()).

2018-04-17 Thread Andrew Lunn 
On Wed, Apr 18, 2018 at 08:32:25AM +1200, Michael Schmitz wrote:
> Hi Adrian,
> 
> On Tue, Apr 17, 2018 at 11:40 PM, John Paul Adrian Glaubitz
>  wrote:
> > On 04/17/2018 04:08 AM, Michael Schmitz wrote:
> >>
> >> From: John Paul Adrian Glaubitz 
> >
> > This should be:
> >
> > From: Michael Karcher 
> 
> I haven't found a way to change that in my tree yet, sorry. Unless
> someone has a simple way to fix patch authorship after a merge, I may
> have to reimport from scratch.

git commit --am --author=

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

Re: [PATCH v2 8/8] net: New ax88796 platform driver for Amiga X-Surf 100 Zorro board (m68k)

2018-04-17 Thread Andrew Lunn 
On Tue, Apr 17, 2018 at 02:08:15PM +1200, Michael Schmitz wrote:
> Add platform device driver to populate the ax88796 platform data from
> information provided by the XSurf100 zorro device driver.
> This driver will have to be loaded before loading the ax88796 module,
> or compiled as built-in.
> 
> Signed-off-by: Michael Karcher 
> Signed-off-by: Michael Schmitz 
> ---
>  drivers/net/ethernet/8390/Kconfig|   14 +-
>  drivers/net/ethernet/8390/Makefile   |1 +
>  drivers/net/ethernet/8390/xsurf100.c |  411 
> ++
>  3 files changed, 425 insertions(+), 1 deletions(-)
>  create mode 100644 drivers/net/ethernet/8390/xsurf100.c
> 
> diff --git a/drivers/net/ethernet/8390/Kconfig 
> b/drivers/net/ethernet/8390/Kconfig
> index fdc6734..0cadd45 100644
> --- a/drivers/net/ethernet/8390/Kconfig
> +++ b/drivers/net/ethernet/8390/Kconfig
> @@ -30,7 +30,7 @@ config PCMCIA_AXNET
>  
>  config AX88796
>   tristate "ASIX AX88796 NE2000 clone support"
> - depends on (ARM || MIPS || SUPERH)
> + depends on (ARM || MIPS || SUPERH || AMIGA)

Hi Michael

Will it compile on other platforms? If so, it is a good idea to add
COMPILE_TEST as well.

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

Re: [PATCH v2 2/8] net: ax88796: Attach MII bus only when open

2018-04-17 Thread Andrew Lunn 
On Tue, Apr 17, 2018 at 02:08:09PM +1200, Michael Schmitz wrote:
> From: Michael Karcher <deb...@mkarcher.dialup.fu-berlin.de>
> 
> Call ax_mii_init in ax_open(), and unregister/remove mdiobus resources
> in ax_close().
> 
> This is needed to be able to unload the module, as the module is busy
> while the MII bus is attached.
> 
> Signed-off-by: Michael Karcher <ker...@mkarcher.dialup.fu-berlin.de>
> Signed-off-by: Michael Schmitz <schmitz...@gmail.com>

Reviewed-by: Andrew Lunn <and...@lunn.ch>

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

Re: [PATCH 08/10] net: ax88796: Make reset more robust on AX88796B

2018-04-17 Thread Andrew Lunn 
On Tue, Apr 17, 2018 at 07:18:10AM +0200, Michael Karcher wrote:
> [Andrew, sorry for the dup. I did hit reply-to-auhor instead of
> reply-to-all first.]
> 
> Andrew Lunn schrieb:
> >> > This should really be fixed in the PHY driver, not the MAC.
> >>
> >> OK - do you want this separate, or as part of this series? Might have
> >> a few side effects on more commonly used hardware, perhaps?
> >
> > Hi Michael
> >
> > What PHY driver is used?
> The ax88796b comes with its own integrated (buggy) PHY needing this
> workaround. This PHY has its own ID which is not known by Linux, so it is
> using the genphy driver as fallback.
> 
> > In the driver you can implement a .soft_reset
> > function which first does the dummy write, and then uses
> > genphy_soft_reset() to do the actual reset.
> We could do that - but I dont't see the point in creating a PHY driver
> that is only ever used by this MAC driver, just to add a single line to
> the genphy driver. If the same PHY might be used with a different MAC,
> you definitely would have a point there, though.


Hi Michael

We try to keep the core code clean, and put all workarounds for buggy
hardware in drivers specific to them. It just helps keep the core code
maintainable.

I would prefer a driver specific to this PHY with the workaround. But
lets see what Florian says.

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

Re: [PATCH 08/10] net: ax88796: Make reset more robust on AX88796B

2018-04-16 Thread Andrew Lunn 
> > This should really be fixed in the PHY driver, not the MAC.
> 
> OK - do you want this separate, or as part of this series? Might have
> a few side effects on more commonly used hardware, perhaps?

Hi Michael

What PHY driver is used? In the driver you can implement a .soft_reset
function which first does the dummy write, and then uses
genphy_soft_reset() to do the actual reset.

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

Re: [PATCH 08/10] net: ax88796: Make reset more robust on AX88796B

2018-04-16 Thread Andrew Lunn 
On Tue, Apr 17, 2018 at 10:04:43AM +1200, Michael Schmitz wrote:
> From: John Paul Adrian Glaubitz 
> 
> The AX88796B as installed on the X-Surf-100 does not recognize a MII reset
> request if the previous write to the MII control register also was a reset
> request. So a dummy write to the control register makes the soft reset in
> the PHY initialization code work.
> 
> Signed-off-by: Michael Karcher 
> ---
>  drivers/net/ethernet/8390/ax88796.c |4 
>  1 files changed, 4 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/net/ethernet/8390/ax88796.c 
> b/drivers/net/ethernet/8390/ax88796.c
> index 6af9aca..a2f9a09 100644
> --- a/drivers/net/ethernet/8390/ax88796.c
> +++ b/drivers/net/ethernet/8390/ax88796.c
> @@ -374,6 +374,10 @@ static int ax_mii_probe(struct net_device *dev)
>   return -ENODEV;
>   }
>  
> + /* write a non-reset pattern to the control register to
> +  * re-arm the reset request detection logic (needed on AX88796B)
> +  */
> + phy_write(phy_dev, MII_BMCR, 0);

This should really be fixed in the PHY driver, not the MAC.

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

Re: [PATCH 02/10] net: ax88796: Attach MII bus only when open

2018-04-16 Thread Andrew Lunn 
On Tue, Apr 17, 2018 at 10:04:37AM +1200, Michael Schmitz wrote:
> From: Michael Karcher 
> 
> Call ax_mii_init in ax_open(), and unregister/remove mdiobus resources
> in ax_close().
> 
> This is needed to be able to unload the module, as the module is busy
> while the MII bus is attached.
> 
> Signed-off-by: Michael Karcher 
> Signed-off-by: Michael Schmitz 
> ---
>  drivers/net/ethernet/8390/ax88796.c |   13 +
>  1 files changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/8390/ax88796.c 
> b/drivers/net/ethernet/8390/ax88796.c
> index 2a256aa..f7b8911 100644
> --- a/drivers/net/ethernet/8390/ax88796.c
> +++ b/drivers/net/ethernet/8390/ax88796.c
> @@ -79,6 +79,8 @@
>  
>  static u32 ax_msg_enable;
>  
> +static int ax_mii_init(struct net_device *dev);

Hi Michael

We try to avoid forward declarations. Please can you move
ax_mii_init() so this is not needed.

> +
>  /* device private data */
>  
>  struct ax_device {
> @@ -396,6 +398,10 @@ static int ax_open(struct net_device *dev)
>  
>   netdev_dbg(dev, "open\n");
>  
> + ret = ax_mii_init(dev);
> + if (ret)
> + goto failed_request_irq;
> +
>   ret = request_irq(dev->irq, ax_ei_interrupt, ax->irqflags,
> dev->name, dev);
>   if (ret)

You are missing some cleanup on error at the end of ax_open().
It was also missing before.

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

Re: [PATCH] phy: remove irq param to fix crash in fixed_phy_add()

2016-05-16 Thread Andrew Lunn 
On Mon, May 16, 2016 at 03:11:35PM +0200, Rabin Vincent wrote:
> On Mon, May 16, 2016 at 02:29:03PM +0200, Andrew Lunn wrote:
> > What i think is better is to make fixed_phy_add() return -EPROBE_DEFER
> > if it is called before fixed_mdio_bus_init().
> 
> I don't see how this will work for platforms such as ar7 and bcm47xx
> which call fixed_phy_add() from platform code.

Ah! Not good.

fixed_phy_add() is the lower layer call. What we can do is only access
fmb->mii_bus->irq[phy_addr] if irq != PHY_POLL. That should make ar7
and bcm47xx work again.

The higher level function fixed_phy_register() should return
-EPROBE_DEFER if fixed_mdio_bus_init() has not been called yet.

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

Re: [PATCH] phy: remove irq param to fix crash in fixed_phy_add()

2016-05-16 Thread Andrew Lunn 
On Mon, May 16, 2016 at 01:15:56PM +0200, Rabin Vincent wrote:
> From: Rabin Vincent 
> 
> Since e7f4dc3536a ("mdio: Move allocation of interrupts into core"),
> platforms which call fixed_phy_add() before fixed_mdio_bus_init() is
> called (for example, because the platform code and the fixed_phy driver
> use the same initcall level) crash in fixed_phy_add() since the
> ->mii_bus is not allocated.
> 
> Also since e7f4dc3536a, these interrupts are initalized to polling by
> default.  All callers of both fixed_phy_register() and fixed_phy_add()
> pass PHY_POLL for the irq argument, so we can fix these crashes by
> simply removing the irq parameter, since the default is correct for all
> users.

Hi Rabin

Thanks for the patch. However, this is more of a work around than a
fix. And it leaves one case still open for bad things to happen. Take
the call sequence:

ret = of_phy_register_fixed_link(port_dn);
if (ret) {
netdev_err(slave_dev, "failed to register fixed PHY: %d\n", 
ret);
return ret;
}

p->phy = of_phy_connect(slave_dev, phy_dn,
dsa_slave_adjust_link,
phy_flags,
p->phy_interface);
 
This is taken from net/dsa/slave.c

With your patch, of_phy_register_fixed_link() will be successful, but
the call to of_phy_connect() will fail, returning NULL, because the
mdio bus the phy is on has not yet been added to the bus.

What i think is better is to make fixed_phy_add() return -EPROBE_DEFER
if it is called before fixed_mdio_bus_init().

I also think it is a bad idea to remove the interrupt
parameter. fixed_phy can actually change it state, so maybe at some
point in the future, somebody will want an interrupt for this? We
should try to keep this phy emulation as similar to real phys as
possible, so lets keep the interrupt for the moment.

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

Re: [PATCH 06/44] gpio-poweroff: Drop reference to pm_power_off from devicetree bindings

2014-10-07 Thread Andrew Lunn 
On Mon, Oct 06, 2014 at 10:28:08PM -0700, Guenter Roeck wrote:
 pm_power_off is an implementation detail. Replace it with a more generic
 description of the driver's functionality.
 
 Cc: Rob Herring robh...@kernel.org
 Cc: Pawel Moll pawel.m...@arm.com
 Cc: Mark Rutland mark.rutl...@arm.com
 Signed-off-by: Guenter Roeck li...@roeck-us.net

Acked-by: Andrew Lunn and...@lunn.ch

Thanks
Andrew
 ---
  Documentation/devicetree/bindings/gpio/gpio-poweroff.txt | 10 +-
  1 file changed, 5 insertions(+), 5 deletions(-)
 
 diff --git a/Documentation/devicetree/bindings/gpio/gpio-poweroff.txt 
 b/Documentation/devicetree/bindings/gpio/gpio-poweroff.txt
 index d4eab92..c95a1a6 100644
 --- a/Documentation/devicetree/bindings/gpio/gpio-poweroff.txt
 +++ b/Documentation/devicetree/bindings/gpio/gpio-poweroff.txt
 @@ -2,12 +2,12 @@ Driver a GPIO line that can be used to turn the power off.
  
  The driver supports both level triggered and edge triggered power off.
  At driver load time, the driver will request the given gpio line and
 -install a pm_power_off handler. If the optional properties 'input' is
 -not found, the GPIO line will be driven in the inactive
 +install a handler to power off the system. If the optional properties
 +'input' is not found, the GPIO line will be driven in the inactive
  state. Otherwise its configured as an input.
  
 -When the pm_power_off is called, the gpio is configured as an output,
 -and drive active, so triggering a level triggered power off
 +When the the poweroff handler is called, the gpio is configured as an
 +output, and drive active, so triggering a level triggered power off
  condition. This will also cause an inactive-active edge condition, so
  triggering positive edge triggered power off. After a delay of 100ms,
  the GPIO is set to inactive, thus causing an active-inactive edge,
 @@ -24,7 +24,7 @@ Required properties:
  
  Optional properties:
  - input : Initially configure the GPIO line as an input. Only reconfigure
 -  it to an output when the pm_power_off function is called. If this optional
 +  it to an output when the poweroff handler is called. If this optional
property is not specified, the GPIO is initialized as an output in its
inactive state.
  
 -- 
 1.9.1
 
 
 ___
 linux-arm-kernel mailing list
 linux-arm-ker...@lists.infradead.org
 http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
To unsubscribe from this list: send the line unsubscribe linux-m68k in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 07/44] qnap-poweroff: Drop reference to pm_power_off from devicetree bindings

2014-10-07 Thread Andrew Lunn 
On Mon, Oct 06, 2014 at 10:28:09PM -0700, Guenter Roeck wrote:
 Replace reference to pm_power_off (which is an implementation detail)
 and replace it with a more generic description of the driver's functionality.

Acked-by: Andrew Lunn and...@lunn.ch

Thanks
Andrew

 
 Cc: Rob Herring robh...@kernel.org
 Cc: Pawel Moll pawel.m...@arm.com
 Cc: Mark Rutland mark.rutl...@arm.com
 Signed-off-by: Guenter Roeck li...@roeck-us.net
 ---
  Documentation/devicetree/bindings/power_supply/qnap-poweroff.txt | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)
 
 diff --git a/Documentation/devicetree/bindings/power_supply/qnap-poweroff.txt 
 b/Documentation/devicetree/bindings/power_supply/qnap-poweroff.txt
 index af25e77..1e2260a 100644
 --- a/Documentation/devicetree/bindings/power_supply/qnap-poweroff.txt
 +++ b/Documentation/devicetree/bindings/power_supply/qnap-poweroff.txt
 @@ -3,8 +3,8 @@
  QNAP NAS devices have a microcontroller controlling the main power
  supply. This microcontroller is connected to UART1 of the Kirkwood and
  Orion5x SoCs. Sending the character 'A', at 19200 baud, tells the
 -microcontroller to turn the power off. This driver adds a handler to
 -pm_power_off which is called to turn the power off.
 +microcontroller to turn the power off. This driver installs a handler
 +to power off the system.
  
  Synology NAS devices use a similar scheme, but a different baud rate,
  9600, and a different character, '1'.
 -- 
 1.9.1
 
 
 ___
 linux-arm-kernel mailing list
 linux-arm-ker...@lists.infradead.org
 http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
To unsubscribe from this list: send the line unsubscribe linux-m68k in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 21/44] power/reset: gpio-poweroff: Register with kernel poweroff handler

2014-10-07 Thread Andrew Lunn 
On Mon, Oct 06, 2014 at 10:28:23PM -0700, Guenter Roeck wrote:
 Register with kernel poweroff handler instead of setting pm_power_off
 directly. Register with a low priority value of 64 to reflect that
 the original code only sets pm_power_off if it was not already set.
 
 Other changes:
 
 Drop note that there can not be an additional instance of this driver.
 The original reason no longer applies, it should be obvious that there
 can only be one instance of the driver if static variables are used to
 reflect its state, and support for multiple instances can now be added
 easily if needed by avoiding static variables.
 
 Do not create an error message if another poweroff handler has already been
 registered. This is perfectly normal and acceptable.
 
 Do not display a warning traceback if the poweroff handler fails to
 power off the system. There may be other poweroff handlers.

I would prefer to keep the warning traceback. We found on some
hardware the GPIO transitions were too fast and it failed to power
off. Seeing the traceback gives an idea where to go look for the
problem.

Other than that,

Acked-by: Andrew Lunn and...@lunn.ch

 
 Cc: Sebastian Reichel s...@kernel.org
 Cc: Dmitry Eremin-Solenikov dbarysh...@gmail.com
 Cc: David Woodhouse dw...@infradead.org
 Signed-off-by: Guenter Roeck li...@roeck-us.net
 ---
  drivers/power/reset/gpio-poweroff.c | 36 ++--
  1 file changed, 18 insertions(+), 18 deletions(-)
 
 diff --git a/drivers/power/reset/gpio-poweroff.c 
 b/drivers/power/reset/gpio-poweroff.c
 index ce849bc..e95a7a1 100644
 --- a/drivers/power/reset/gpio-poweroff.c
 +++ b/drivers/power/reset/gpio-poweroff.c
 @@ -14,18 +14,18 @@
  #include linux/kernel.h
  #include linux/init.h
  #include linux/delay.h
 +#include linux/notifier.h
 +#include linux/pm.h
  #include linux/platform_device.h
  #include linux/gpio/consumer.h
  #include linux/of_platform.h
  #include linux/module.h
  
 -/*
 - * Hold configuration here, cannot be more than one instance of the driver
 - * since pm_power_off itself is global.
 - */
  static struct gpio_desc *reset_gpio;
  
 -static void gpio_poweroff_do_poweroff(void)
 +static int gpio_poweroff_do_poweroff(struct notifier_block *this,
 +  unsigned long unused1, void *unused2)
 +
  {
   BUG_ON(!reset_gpio);
  
 @@ -42,20 +42,18 @@ static void gpio_poweroff_do_poweroff(void)
   /* give it some time */
   mdelay(3000);
  
 - WARN_ON(1);
 + return NOTIFY_DONE;
  }
  
 +static struct notifier_block gpio_poweroff_nb = {
 + .notifier_call = gpio_poweroff_do_poweroff,
 + .priority = 64,
 +};
 +
  static int gpio_poweroff_probe(struct platform_device *pdev)
  {
   bool input = false;
 -
 - /* If a pm_power_off function has already been added, leave it alone */
 - if (pm_power_off != NULL) {
 - dev_err(pdev-dev,
 - %s: pm_power_off function already registered,
 -__func__);
 - return -EBUSY;
 - }
 + int err;
  
   reset_gpio = devm_gpiod_get(pdev-dev, NULL);
   if (IS_ERR(reset_gpio))
 @@ -77,14 +75,16 @@ static int gpio_poweroff_probe(struct platform_device 
 *pdev)
   }
   }
  
 - pm_power_off = gpio_poweroff_do_poweroff;
 - return 0;
 + err = register_poweroff_handler(gpio_poweroff_nb);
 + if (err)
 + dev_err(pdev-dev, Failed to register poweroff handler\n);
 +
 + return err;
  }
  
  static int gpio_poweroff_remove(struct platform_device *pdev)
  {
 - if (pm_power_off == gpio_poweroff_do_poweroff)
 - pm_power_off = NULL;
 + unregister_poweroff_handler(gpio_poweroff_nb);
  
   return 0;
  }
 -- 
 1.9.1
 
 
 ___
 linux-arm-kernel mailing list
 linux-arm-ker...@lists.infradead.org
 http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
To unsubscribe from this list: send the line unsubscribe linux-m68k in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 23/44] power/reset: qnap-poweroff: Register with kernel poweroff handler

2014-10-07 Thread Andrew Lunn 
On Mon, Oct 06, 2014 at 10:28:25PM -0700, Guenter Roeck wrote:
 Register with kernel poweroff handler instead of setting pm_power_off
 directly. Register with default priority value of 128 to reflect that
 the original code generates an error if another poweroff handler has
 already been registered when the driver is loaded.
 
 Cc: Sebastian Reichel s...@kernel.org
 Cc: Dmitry Eremin-Solenikov dbarysh...@gmail.com
 Cc: David Woodhouse dw...@infradead.org
 Signed-off-by: Guenter Roeck li...@roeck-us.net

Acked-by: Andrew Lunn and...@lunn.ch

Thanks
Andrew

 ---
  drivers/power/reset/qnap-poweroff.c | 28 ++--
  1 file changed, 14 insertions(+), 14 deletions(-)
 
 diff --git a/drivers/power/reset/qnap-poweroff.c 
 b/drivers/power/reset/qnap-poweroff.c
 index a75db7f..c474980 100644
 --- a/drivers/power/reset/qnap-poweroff.c
 +++ b/drivers/power/reset/qnap-poweroff.c
 @@ -16,7 +16,9 @@
  
  #include linux/kernel.h
  #include linux/module.h
 +#include linux/notifier.h
  #include linux/platform_device.h
 +#include linux/pm.h
  #include linux/serial_reg.h
  #include linux/kallsyms.h
  #include linux/of.h
 @@ -55,7 +57,8 @@ static void __iomem *base;
  static unsigned long tclk;
  static const struct power_off_cfg *cfg;
  
 -static void qnap_power_off(void)
 +static int qnap_power_off(struct notifier_block *this, unsigned long unused1,
 +   void *unused2)
  {
   const unsigned divisor = ((tclk + (8 * cfg-baud)) / (16 * cfg-baud));
  
 @@ -72,14 +75,20 @@ static void qnap_power_off(void)
  
   /* send the power-off command to PIC */
   writel(cfg-cmd, UART1_REG(TX));
 +
 + return NOTIFY_DONE;
  }
  
 +static struct notifier_block qnap_poweroff_nb = {
 + .notifier_call = qnap_power_off,
 + .priority = 128,
 +};
 +
  static int qnap_power_off_probe(struct platform_device *pdev)
  {
   struct device_node *np = pdev-dev.of_node;
   struct resource *res;
   struct clk *clk;
 - char symname[KSYM_NAME_LEN];
  
   const struct of_device_id *match =
   of_match_node(qnap_power_off_of_match_table, np);
 @@ -106,22 +115,13 @@ static int qnap_power_off_probe(struct platform_device 
 *pdev)
  
   tclk = clk_get_rate(clk);
  
 - /* Check that nothing else has already setup a handler */
 - if (pm_power_off) {
 - lookup_symbol_name((ulong)pm_power_off, symname);
 - dev_err(pdev-dev,
 - pm_power_off already claimed %p %s,
 - pm_power_off, symname);
 - return -EBUSY;
 - }
 - pm_power_off = qnap_power_off;
 -
 - return 0;
 + return register_poweroff_handler(qnap_poweroff_nb);
  }
  
  static int qnap_power_off_remove(struct platform_device *pdev)
  {
 - pm_power_off = NULL;
 + unregister_poweroff_handler(qnap_poweroff_nb);
 +
   return 0;
  }
  
 -- 
 1.9.1
 
 
 ___
 linux-arm-kernel mailing list
 linux-arm-ker...@lists.infradead.org
 http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
To unsubscribe from this list: send the line unsubscribe linux-m68k in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 20/44] power/reset: restart-poweroff: Register with kernel poweroff handler

2014-10-07 Thread Andrew Lunn 
On Mon, Oct 06, 2014 at 10:28:22PM -0700, Guenter Roeck wrote:
 Register with kernel poweroff handler instead of seting pm_power_off
 directly.  Register as poweroff handler of last resort since the driver
 does not really power off the system but executes a restart.

I would not say last resort, this is how it is designed to work. There
is no way to turn the power off from with linux, it is designed that
u-boot will put the hardware into minimal power consumption until the
power button is pressed.

Other than that, 

Acked-by: Andrew Lunn and...@lunn.ch

Thanks
Andrew

 
 Cc: Sebastian Reichel s...@kernel.org
 Cc: Dmitry Eremin-Solenikov dbarysh...@gmail.com
 Cc: David Woodhouse dw...@infradead.org
 Signed-off-by: Guenter Roeck li...@roeck-us.net
 ---
  drivers/power/reset/restart-poweroff.c | 25 -
  1 file changed, 12 insertions(+), 13 deletions(-)
 
 diff --git a/drivers/power/reset/restart-poweroff.c 
 b/drivers/power/reset/restart-poweroff.c
 index edd707e..5437697 100644
 --- a/drivers/power/reset/restart-poweroff.c
 +++ b/drivers/power/reset/restart-poweroff.c
 @@ -12,35 +12,34 @@
   */
  #include linux/kernel.h
  #include linux/init.h
 +#include linux/notifier.h
  #include linux/platform_device.h
  #include linux/of_platform.h
  #include linux/module.h
 +#include linux/pm.h
  #include linux/reboot.h
 -#include asm/system_misc.h
  
 -static void restart_poweroff_do_poweroff(void)
 +static int restart_poweroff_do_poweroff(struct notifier_block *this,
 + unsigned long unused1, void *unused2)
  {
   reboot_mode = REBOOT_HARD;
   machine_restart(NULL);
 +
 + return NOTIFY_DONE;
  }
  
 +static struct notifier_block restart_poweroff_handler = {
 + .notifier_call = restart_poweroff_do_poweroff,
 +};
 +
  static int restart_poweroff_probe(struct platform_device *pdev)
  {
 - /* If a pm_power_off function has already been added, leave it alone */
 - if (pm_power_off != NULL) {
 - dev_err(pdev-dev,
 - pm_power_off function already registered);
 - return -EBUSY;
 - }
 -
 - pm_power_off = restart_poweroff_do_poweroff;
 - return 0;
 + return register_poweroff_handler(restart_poweroff_handler);
  }
  
  static int restart_poweroff_remove(struct platform_device *pdev)
  {
 - if (pm_power_off == restart_poweroff_do_poweroff)
 - pm_power_off = NULL;
 + unregister_poweroff_handler(restart_poweroff_handler);
  
   return 0;
  }
 -- 
 1.9.1
 
 
 ___
 linux-arm-kernel mailing list
 linux-arm-ker...@lists.infradead.org
 http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
To unsubscribe from this list: send the line unsubscribe linux-m68k in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html