Re: [PATCH 2/2] net: mdio: support c45 peripherals on c22 busses

[Danilo Krummrich]

I somehow missed this mail..

On Mon, Apr 05, 2021 at 10:12:36PM +0100, Russell King - ARM Linux admin wrote:
> On Mon, Apr 05, 2021 at 08:58:07PM +0200, Danilo Krummrich wrote:
> > On Mon, Apr 05, 2021 at 03:33:55PM +0200, Andrew Lunn wrote:
> > However, this was about something else - Russell wrote:
> > > > > We have established that MDIO drivers need to reject accesses for
> > > > > reads/writes that they do not support [..]
> > The MDIO drivers do this by checking the MII_ADDR_C45 flag if it's a C45 bus
> > request. In case they don't support it they return -EOPNOTSUPP. So 
> > basically,
> > the bus drivers read/write functions (should) encode the capability of doing
> > C45 transfers.
> > 
> > I just noted that this is redundant to the bus' capabilities field of
> > struct mii_bus which also encodes the bus' capabilities of doing C22 and/or 
> > C45
> > transfers.
> > 
> > Now, instead of encoding this information of the bus' capabilities at both
> > places, I'd propose just checking the mii_bus->capabilities field in the
> > mdiobus_c45_*() functions. IMHO this would be a little cleaner, than having 
> > two
> > places where this information is stored. What do you think about that?
> 
> It would be good to clean that up, but that's a lot of work given the
> number of drivers - not only in terms of making the changes but also
> making sure that the changes are as correct as would be sensibly
> achievable... then there's the problem of causing regressions by doing
> so.
> 
As mentioned in the answer to Andrew, I think it wouldn't be too many as we'd
only need to look for the ones considering MII_ADDR_C45 at all.
> The two ways were introduced at different times to do two different
> things: the checking in the read/write methods in each driver was the
> first method, which was being added to newer drivers. Then more
> recently came the ->capabilities field.
> 
> So now we have some drivers that:
> - do no checks and don't fill in ->capabilities either (some of which
>   are likely C22-only.)
Guess they could be left untouched completely and simply go for MDIOBUS_NO_CAP
implicitly and therefore C45 requests would be correctly rejected by
mdiobus_c45_*() functions. I think this would be the main advantage apart from
making it more clean.
> - check in their read/write methods for access types they don't support
>   (e.g. drivers/net/ethernet/marvell/mvmdio.c) and don't fill in
>   ->capabilities. Note, mvmdio supports both C22-only and C45-only
>   interfaces with no C22-and-C45 interfaces.
Yes, I think the correct capability could still be easily assigned per instance
within the probe() function:
switch (type) {
case BUS_TYPE_SMI:
bus->capabilities = MDIOBUS_C22;
[...]
break;
case BUS_TYPE_XSMI:
bus->capabilities = MDIOBUS_C45;
[...]
break;
}
> - do fill in ->capabilities with MDIOBUS_C22_C45 (and hence have no
>   checks in their read/write functions).
> 
Nothing to be done for them.
> Sometimes, its best to leave stuff alone... if it ain't broke, don't
> make regressions.
> 
Yes, I can perfectly understand that.

However, maybe I go for a patch anyways, and if so I will also send it. If it's
taken by you in the end is on another piece of paper.

Thanks again!

Danilo

> -- 
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH 2/2] net: mdio: support c45 peripherals on c22 busses

[Danilo Krummrich]

On Tue, Apr 06, 2021 at 12:30:11AM +0200, Danilo Krummrich wrote:
> On Mon, Apr 05, 2021 at 09:27:44PM +0200, Andrew Lunn wrote:
> > > Now, instead of encoding this information of the bus' capabilities at both
> > > places, I'd propose just checking the mii_bus->capabilities field in the
> > > mdiobus_c45_*() functions. IMHO this would be a little cleaner, than 
> > > having two
> > > places where this information is stored. What do you think about that?
> > 
> > You will need to review all the MDIO bus drivers to make sure they
> > correctly set the capabilities. There is something like 55 using
> > of_mdiobus_register() and 45 using mdiobus_register(). So you have 100
> > drivers to review.
> Yes, but I think it would be enough to look at the drivers handling the
> MII_ADDR_C45 flag, because those are either
> - actually capable to do C45 bus transfers or
> - do properly return -EOPNOTSUPP.
> 
> I counted 27 drivers handling the MII_ADDR_C45 flag. Setting the capabilities
> for those should be pretty easy.
> 
> The remaining ones, which should be about 73 then, could be left untouched,
> because the default capability MDIOBUS_NO_CAP would indicate they can C22 
> only.
> Since they don't handle the MII_ADDR_C45 flag at all, this should be the
> correct assumption.
Forgot to mention, this would also automatically fixup that userspace C45
requests for those "remaining" drivers results in garbage on the bus. :-)
> > 
> > Andrew
> > 

Re: [PATCH 2/2] net: mdio: support c45 peripherals on c22 busses

[Danilo Krummrich]

On Mon, Apr 05, 2021 at 09:27:44PM +0200, Andrew Lunn wrote:
> > Now, instead of encoding this information of the bus' capabilities at both
> > places, I'd propose just checking the mii_bus->capabilities field in the
> > mdiobus_c45_*() functions. IMHO this would be a little cleaner, than having 
> > two
> > places where this information is stored. What do you think about that?
> 
> You will need to review all the MDIO bus drivers to make sure they
> correctly set the capabilities. There is something like 55 using
> of_mdiobus_register() and 45 using mdiobus_register(). So you have 100
> drivers to review.
Yes, but I think it would be enough to look at the drivers handling the
MII_ADDR_C45 flag, because those are either
- actually capable to do C45 bus transfers or
- do properly return -EOPNOTSUPP.

I counted 27 drivers handling the MII_ADDR_C45 flag. Setting the capabilities
for those should be pretty easy.

The remaining ones, which should be about 73 then, could be left untouched,
because the default capability MDIOBUS_NO_CAP would indicate they can C22 only.
Since they don't handle the MII_ADDR_C45 flag at all, this should be the
correct assumption.
> 
>   Andrew
> 

Re: [PATCH 2/2] net: mdio: support c45 peripherals on c22 busses

[Danilo Krummrich]

On Mon, Apr 05, 2021 at 03:33:55PM +0200, Andrew Lunn wrote:
> On Sun, Apr 04, 2021 at 09:23:55PM +0200, Danilo Krummrich wrote:
> > So currently every driver should check for the flag MII_ADDR_C45 and report 
> > an
> > error in case it's unsupported.
> > 
> > What do you think about checking the bus' capabilities instead in
> > mdiobus_c45_*()? This way the check if C45 is supported can even happen 
> > before
> > calling the driver at all. I think that would be a little cleaner than 
> > having
> > two places where information of the bus' capabilities are stored (return 
> > value
> > of read/write functions and the capabilities field).
> > 
> > I think there are not too many drivers setting their capabilities though, 
> > but
> > it should be easy to derive this information from how and if they handle the
> > MII_ADDR_C45 flag.
> 
> I actually don't think anything needs to change. The Marvell PHY
> probably probes due to its C22 IDs. The driver will then requests C45
> access which automagically get converted into C45 over C22 for your
> hardware, but remain C45 access for bus drivers which support C45.
> 
Thanks Andrew - I agree, for the Marvell PHY to work I likly don't need any
change, since I also expect that it will probe with the C22 IDs. I'll try
this soon.

However, this was about something else - Russell wrote:
> > > We have established that MDIO drivers need to reject accesses for
> > > reads/writes that they do not support [..]
The MDIO drivers do this by checking the MII_ADDR_C45 flag if it's a C45 bus
request. In case they don't support it they return -EOPNOTSUPP. So basically,
the bus drivers read/write functions (should) encode the capability of doing
C45 transfers.

I just noted that this is redundant to the bus' capabilities field of
struct mii_bus which also encodes the bus' capabilities of doing C22 and/or C45
transfers.

Now, instead of encoding this information of the bus' capabilities at both
places, I'd propose just checking the mii_bus->capabilities field in the
mdiobus_c45_*() functions. IMHO this would be a little cleaner, than having two
places where this information is stored. What do you think about that?
> Andrew

Re: [PATCH 2/2] net: mdio: support c45 peripherals on c22 busses

[Danilo Krummrich]

On Fri, Apr 02, 2021 at 01:58:58PM +0100, Russell King - ARM Linux admin wrote:
> On Fri, Apr 02, 2021 at 03:10:49AM +0200, Danilo Krummrich wrote:
> > On Thu, Apr 01, 2021 at 09:48:58AM +0100, Russell King - ARM Linux admin 
> > wrote:
> > > One could also argue this is a feature, and it allows userspace to
> > > know whether C45 cycles are supported or not.
> > >
> > No, if the userspace requests such a transfer although the MDIO controller
> > does not support real c45 framing the kernel will call mdiobus_c45_addr() to
> > join the devaddr and  and regaddr in one parameter and pass it to
> > mdiobus_read() or mdiobus_write(). A bus driver not supporting c45 framing
> > will not care and just mask/shift the joined value and write it to the
> > particular register. Obviously, this will result into complete garbage being
> > read or (even worse) written.
> 
> 
> We have established that MDIO drivers need to reject accesses for
> reads/writes that they do not support - this isn't something that
> they have historically checked for because it is only recent that
> phylib has really started to support clause 45 PHYs.
> 
I see, that's why you consider it a feature - because it is.
What do you think about adding a flag MDIO_PHY_ID_MMD (or similar) analog to
MDIO_PHY_ID_C45 for phy_mii_ioctl() to check for, such that userspace can ask
for an indirect access in order to save userspace doing the indirect access
itself. A nice side effect would be saving 3 syscalls per request.
> More modern MDIO drivers check the requested access type and error
> out - we need the older MDIO drivers to do the same.
> 
So currently every driver should check for the flag MII_ADDR_C45 and report an
error in case it's unsupported.

What do you think about checking the bus' capabilities instead in
mdiobus_c45_*()? This way the check if C45 is supported can even happen before
calling the driver at all. I think that would be a little cleaner than having
two places where information of the bus' capabilities are stored (return value
of read/write functions and the capabilities field).

I think there are not too many drivers setting their capabilities though, but
it should be easy to derive this information from how and if they handle the
MII_ADDR_C45 flag.

Re: [PATCH 2/2] net: mdio: support c45 peripherals on c22 busses

[Danilo Krummrich]

On Fri, Apr 02, 2021 at 02:28:54PM +0200, Andrew Lunn wrote:
> > > Do you actually have a requirement for this?
> > >
> > Yes, the Marvell 88Q2112 1000Base-T1 PHY. But actually, I just recognize 
> > that it
> > should be possible to just register it with the compatible string
> > "ethernet-phy-ieee802.3-c22" instead of "ethernet-phy-ieee802.3-c45", this
> > should result in probing it as c22 PHY and doing indirect accesses through
> > phy_*_mmd().
> 
> Hi Danilo
> 
> Do you plan to submit a driver for this?
> 
Hi Andrew,

Yes, I'll get it ready once I got some spare time.
> Does this device have an ID in register 2 and 3 in C22 space?
> 
Currently, I don't have access to the datasheet and I don't remember.
In a couple of days I should have access to the HW again and I will try.
>  Andrew
Cheers,
Danilo

Re: [PATCH 2/2] net: mdio: support c45 peripherals on c22 busses

[Danilo Krummrich]

On Thu, Apr 01, 2021 at 09:48:58AM +0100, Russell King - ARM Linux admin wrote:
> On Thu, Apr 01, 2021 at 03:23:05AM +0200, danilokrummr...@dk-develop.de wrote:
> > On 2021-03-31 20:35, Russell King - ARM Linux admin wrote:
> > > On Wed, Mar 31, 2021 at 07:58:33PM +0200, danilokrummr...@dk-develop.de
> > > wrote:
> > > > For this cited change the only thing happening is that if
> > > > get_phy_device()
> > > > already failed for probing with is_c45==false (C22 devices) it tries
> > > > to
> > > > probe with is_c45==true (C45 devices) which then either results into
> > > > actual
> > > > C45 frame transfers or indirect accesses by calling mdiobus_c45_*()
> > > > functions.
> > >
> > > Please explain why and how a PHY may not appear to be present using
> > > C22 frames to read the ID registers, but does appear to be present
> > > when using C22 frames to the C45 indirect registers - and summarise
> > > which PHYs have this behaviour.
> > >
> > > It seems very odd that any PHY would only implement C45 indirect
> > > registers in the C22 register space.
> >
> > Honestly, I can't list examples of that case (at least none that have an
> > upstream driver already). This part of my patch, to fall back to c45 bus
> > probing when c22 probing does not succeed, is also motivated by the fact
> > that this behaviour was already introduced with this patch:
>
> So, if I understand what you've just said, you want to make a change to
> the kernel to add support for something that you don't need and don't
> know that there's any hardware that needs it.  Is that correct?
>
No, not at all. As I explained this part of the patch in mdiobus_scan() I did
based on the patch of Jeremy only. It was an indicator for me that there might
be c45 PHYs that don't respond to c22 requests. I interpreted his commit
message in a way that those c45 PHYs are capable of processing c22 requests in
general but implement the indirect registers only, since he said "its possible
that a c45 device doesn't respond despite being a standard phy".

You said that this behaviour would be very odd and I agree. Now, likely I just
misinterpreted this and Jeremy actually tells that the PHYs he's referring to
don't support c22 access at all. In this case we can for sure just forget
about the changes in mdiobus_scan() of this patch. I'll remove them.

Again, just to sort this out, this part of the patch is not it's main purpose.
However, since I implemented the fallback to indirect accesses anyways I
thought it doesn't hurt to consider the case that a PHY implements indirect
access registers only. Anyways, I admit that this is likely pointless and as
said, I'll remove it from the patch.
> > commit 0cc8fecf041d3e5285380da62cc6662bdc942d8c
> > Author: Jeremy Linton 
> > Date:   Mon Jun 22 20:35:32 2020 +0530
> >
> > net: phy: Allow mdio buses to auto-probe c45 devices
> >
> > The mdiobus_scan logic is currently hardcoded to only
> > work with c22 devices. This works fairly well in most
> > cases, but its possible that a c45 device doesn't respond
> > despite being a standard phy. If the parent hardware
> > is capable, it makes sense to scan for c22 devices before
> > falling back to c45.
> >
> > As we want this to reflect the capabilities of the STA,
> > lets add a field to the mii_bus structure to represent
> > the capability. That way devices can opt into the extended
> > scanning. Existing users should continue to default to c22
> > only scanning as long as they are zero'ing the structure
> > before use.
> >
> > Signed-off-by: Jeremy Linton 
> > Signed-off-by: Calvin Johnson 
> > Signed-off-by: David S. Miller 
> >
> > In this patch i.a. the following lines were added.
> >
> > +   case MDIOBUS_C22_C45:
> > +   phydev = get_phy_device(bus, addr, false);
> > +   if (IS_ERR(phydev))
> > +   phydev = get_phy_device(bus, addr, true);
> > +   break;
> >
> > I'm applying the same logic for MDIOBUS_NO_CAP and MDIOBUS_C22, since
> > with my patch MDIO controllers with those capabilities can handle c45 bus
> > probing with indirect accesses.
>
> If the PHY doesn't respond to C22 accesses but is a C45 PHY, then how
> can this work (you seem to have essentially said it doesn't above.)
>
As stated above likely I wrongly interpreted his commit message as if only the
indirect registers are implemented.
> > [By the way, I'm unsure if this order for MDIO bus controllers with the
> > capability MDIOBUS_C22_C45 makes sense, because if we assume that the
> > majority of c45 PHYs responds well to c22 probing (which I'm convinced of)
>
> There are some which don't - Clause 45 allows PHYs not to implement
> support for Clause 22 accesses.
>
Yes, I agree.
> > the PHY would still be registered as is_c45==false, which results in the
> > fact
> > that even though the underlying bus is capable of real c45 framing only
> > indirect accessing would be performed. But 

net: mdio: support c45 peripherals on c22 only capable mdio controllers

[Danilo Krummrich]

This patch series adds support for clause 45 peripherals on busses driven
by an mdio controller that is only capable of clause 22 frame format
messages by indirect bus accesses.

In order to do so we

- change the name of the field probe_capabilities to capabilities in
  struct mii_bus to represent the bus capabilities in general (not only
  for probing)
- add functions mdiobus_*_mmd() and mdiobus_indirect_mmd() to handle
  indirect bus accesses
- let mdiobus_c45_*() functions check the bus capabilities in order to
  decide whether a real clause 45 bus transfer or indirect access should
  be performed
- use the new functions to simplify existing code a little bit
- and finally allow probing for clause 45 peripherals on busses that are
  not capable of cause 45 frame format

net: mdio: support c45 peripherals on c22 only capable mdio controllers

[Danilo Krummrich]

This patch series adds support for clause 45 peripherals on busses driven
by an mdio controller that is only capable of clause 22 frame format
messages by indirect bus accesses.

In order to do so we

- change the name of the field probe_capabilities to capabilities in
  struct mii_bus to represent the bus capabilities in general (not only
  for probing)
- add functions mdiobus_*_mmd() and mdiobus_indirect_mmd() to handle
  indirect bus accesses
- let mdiobus_c45_*() functions check the bus capabilities in order to
  decide whether a real clause 45 bus transfer or indirect access should
  be performed
- use the new functions to simplify existing code a little bit
- and finally allow probing for clause 45 peripherals on busses that are
  not capable of the clause 45 frame format

[PATCH 1/2] net: mdio: rename mii bus probe_capabilities

[Danilo Krummrich]

Rename the probe_capabilities field of struct mii_bus to capabilities.

This field represents the supported frame formats of the mdio controller
backing this bus as by IEEE 802.3 in general. This is not specific to the
probing procedure of the bus.

Signed-off-by: Danilo Krummrich 
---
 drivers/net/ethernet/freescale/xgmac_mdio.c   | 2 +-
 drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c | 2 +-
 drivers/net/phy/mdio_bus.c| 2 +-
 include/linux/phy.h   | 7 +--
 4 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/freescale/xgmac_mdio.c 
b/drivers/net/ethernet/freescale/xgmac_mdio.c
index bfa2826c5545..bda04154fca2 100644
--- a/drivers/net/ethernet/freescale/xgmac_mdio.c
+++ b/drivers/net/ethernet/freescale/xgmac_mdio.c
@@ -268,7 +268,7 @@ static int xgmac_mdio_probe(struct platform_device *pdev)
bus->read = xgmac_mdio_read;
bus->write = xgmac_mdio_write;
bus->parent = >dev;
-   bus->probe_capabilities = MDIOBUS_C22_C45;
+   bus->capabilities = MDIOBUS_C22_C45;
snprintf(bus->id, MII_BUS_ID_SIZE, "%pa", >start);
 
/* Set the PHY base address */
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c 
b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
index d64116e0543e..917537731131 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
@@ -366,7 +366,7 @@ int stmmac_mdio_register(struct net_device *ndev)
new_bus->name = "stmmac";
 
if (priv->plat->has_gmac4)
-   new_bus->probe_capabilities = MDIOBUS_C22_C45;
+   new_bus->capabilities = MDIOBUS_C22_C45;
 
if (priv->plat->has_xgmac) {
new_bus->read = _xgmac2_mdio_read;
diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
index 823518554079..d03e40a0fbae 100644
--- a/drivers/net/phy/mdio_bus.c
+++ b/drivers/net/phy/mdio_bus.c
@@ -670,7 +670,7 @@ struct phy_device *mdiobus_scan(struct mii_bus *bus, int 
addr)
struct phy_device *phydev = ERR_PTR(-ENODEV);
int err;
 
-   switch (bus->probe_capabilities) {
+   switch (bus->capabilities) {
case MDIOBUS_NO_CAP:
case MDIOBUS_C22:
phydev = get_phy_device(bus, addr, false);
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 1a12e4436b5b..ba5eb317a471 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -362,13 +362,16 @@ struct mii_bus {
/** @reset_gpiod: Reset GPIO descriptor pointer */
struct gpio_desc *reset_gpiod;
 
-   /** @probe_capabilities: bus capabilities, used for probing */
+   /**
+* @capabilities: bus capabilities, representing supported frame
+* formats as by IEEE 802.3
+*/
enum {
MDIOBUS_NO_CAP = 0,
MDIOBUS_C22,
MDIOBUS_C45,
MDIOBUS_C22_C45,
-   } probe_capabilities;
+   } capabilities;
 
/** @shared_lock: protect access to the shared element */
struct mutex shared_lock;
-- 
2.31.0

[PATCH 2/2] net: mdio: support c45 peripherals on c22 busses

[Danilo Krummrich]

There are still a lot of mdio controllers which don't support the clause
45 frame format as well as drivers for mdio controllers which don't
implement the cause 45 mode of the controller even if natively supported
by the hardware. Therefore it makes sense to support clause 45 peripherals
on busses that support clause 22 transfers only by indirect access.

In order to do so we can use the capabilitiy field of the struct mii_bus
to distinguish between busses that natively support clause 45 and those
who don't. Based on that the mdiobus_c45_*() functions can either issue
a MII_ADDR_C45 flagged request to the bus driver or perform an indirect
access.

The indirect access is performed by the introduced mdiobus_*_mmd()
functions. While performing the indirect access sequence in
mdiobus_indirect_mmd() we check for potential errors occurring in the
sequence, which was not done previously and just assumed to be
successful.

Signed-off-by: Danilo Krummrich 
---
 drivers/net/phy/mdio_bus.c | 265 -
 drivers/net/phy/phy-core.c |  46 ++-
 drivers/net/phy/phy.c  |  19 ++-
 include/linux/mdio.h   |  36 ++---
 4 files changed, 298 insertions(+), 68 deletions(-)

diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
index d03e40a0fbae..c80ed65666ac 100644
--- a/drivers/net/phy/mdio_bus.c
+++ b/drivers/net/phy/mdio_bus.c
@@ -670,19 +670,21 @@ struct phy_device *mdiobus_scan(struct mii_bus *bus, int 
addr)
struct phy_device *phydev = ERR_PTR(-ENODEV);
int err;
 
+   /* In case of NO_CAP and C22 only, we still can try to scan for C45
+* devices, since indirect access will be used for busses that are not
+* capable of C45 frame format.
+*/
switch (bus->capabilities) {
case MDIOBUS_NO_CAP:
case MDIOBUS_C22:
-   phydev = get_phy_device(bus, addr, false);
-   break;
-   case MDIOBUS_C45:
-   phydev = get_phy_device(bus, addr, true);
-   break;
case MDIOBUS_C22_C45:
phydev = get_phy_device(bus, addr, false);
if (IS_ERR(phydev))
phydev = get_phy_device(bus, addr, true);
break;
+   case MDIOBUS_C45:
+   phydev = get_phy_device(bus, addr, true);
+   break;
}
 
if (IS_ERR(phydev))
@@ -903,6 +905,259 @@ int mdiobus_write(struct mii_bus *bus, int addr, u32 
regnum, u16 val)
 }
 EXPORT_SYMBOL(mdiobus_write);
 
+/**
+ * mdiobus_indirect_mmd - Prepares MMD indirect access
+ * @bus: the mii_bus struct
+ * @addr: the phy address
+ * @devad: the device address
+ * @regnum: register number to read
+ *
+ * Prepares indirect MMD access, such that only the MII_MMD_DATA register is
+ * left to be read or written. Caller must hold the mdio bus lock.
+ *
+ * NOTE: MUST NOT be called from interrupt context.
+ */
+static int mdiobus_indirect_mmd(struct mii_bus *bus, int addr, u16 devad, u32 
regnum)
+{
+   int err;
+
+   /* Write the desired MMD Devad */
+   err = __mdiobus_write(bus, addr, MII_MMD_CTRL, devad);
+   if (err)
+   goto out;
+
+   /* Write the desired MMD register address */
+   err = __mdiobus_write(bus, addr, MII_MMD_DATA, regnum);
+   if (err)
+   goto out;
+
+   /* Select the Function : DATA with no post increment */
+   err = __mdiobus_write(bus, addr, MII_MMD_CTRL,
+ devad | MII_MMD_CTRL_NOINCR);
+
+out:
+   return err;
+}
+
+/**
+ * __mdiobus_read_mmd - Unlocked version of the mdiobus_read_mmd function
+ * @bus: the mii_bus struct
+ * @addr: the phy address
+ * @devad: the device address
+ * @regnum: register number to read
+ *
+ * Read a MDIO bus register. Caller must hold the mdio bus lock.
+ *
+ * NOTE: MUST NOT be called from interrupt context.
+ */
+int __mdiobus_read_mmd(struct mii_bus *bus, int addr, u16 devad, u32 regnum)
+{
+   int retval;
+
+   retval = mdiobus_indirect_mmd(bus, addr, devad, regnum);
+   if (retval)
+   goto out;
+
+   /* Read the content of the MMD's selected register */
+   retval = __mdiobus_read(bus, addr, MII_MMD_DATA);
+
+out:
+   return retval;
+}
+EXPORT_SYMBOL(__mdiobus_read_mmd);
+
+/**
+ * __mdiobus_write_mmd - Unlocked version of the mdiobus_write_mmd function
+ * @bus: the mii_bus struct
+ * @addr: the phy address
+ * @devad: the device address
+ * @regnum: register number to write
+ * @val: value to write to @regnum
+ *
+ * Write a MDIO bus register. Caller must hold the mdio bus lock.
+ *
+ * NOTE: MUST NOT be called from interrupt context.
+ */
+int __mdiobus_write_mmd(struct mii_bus *bus, int addr, u16 devad, u32 regnum,
+   u16 val)
+{
+   int err;
+
+   err = mdiobus_indirect_mmd(bus, addr, devad, regnum);
+   if (err)
+   goto out;
+
+   /* Write the data into MMD's selected register */
+   

Re: [PATCH] Input: ps2-gpio - flush TX work when closing port

[Danilo Krummrich]

On 2019-02-08 08:31, Dmitry Torokhov wrote:

On Thu, Feb 07, 2019 at 06:03:03PM -0500, Sven Van Asbroeck wrote:

On Thu, Feb 7, 2019 at 5:27 PM Dmitry Torokhov
 wrote:
>
> +   flush_work(>tx_work.work);

Would cancel_work_sync() be better than flush_work() ?


No, because we want to have interrupt and gpios in a consistent state.
If we cancel then we need to see if we should disable it or it may
already be disabled, etc. This way we know it is enabled after
flush_delayed_work() returns, and we need to disable it.

Thanks.


I agree with Dmitry - thanks for the fix.

Acked-by: Danilo Krummrich 

Re: [PATCH] Add USB_QUIRK_DELAY_CTRL_MSG quirk for Corsair K70 RGB

[Danilo Krummrich]

You're referring to this patch
cb88a0588717 ("usb: quirks: add control message delay for 1b1c:1b20")
I guess. Maybe you want to name it explicitly for better reference.

Acked-by: Danilo Krummrich 


On 2019-01-02 19:37, Jack Stocker wrote:

To match the Corsair Strafe RGB, the Corsair K70 RGB also requires
USB_QUIRK_DELAY_CTRL_MSG to completely resolve boot connection issues
discussed here: https://github.com/ckb-next/ckb-next/issues/42.
Otherwise roughly 1 in 10 boots the keyboard will fail to be detected.

Signed-off-by: Jack Stocker 
---
 drivers/usb/core/quirks.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/core/quirks.c b/drivers/usb/core/quirks.c
index 7909262..4a9267d 100644
--- a/drivers/usb/core/quirks.c
+++ b/drivers/usb/core/quirks.c
@@ -377,7 +377,8 @@ static const struct usb_device_id usb_quirk_list[] 
= {

USB_QUIRK_LINEAR_UFRAME_INTR_BINTERVAL },

/* Corsair K70 RGB */
-   { USB_DEVICE(0x1b1c, 0x1b13), .driver_info = USB_QUIRK_DELAY_INIT },
+   { USB_DEVICE(0x1b1c, 0x1b13), .driver_info = USB_QUIRK_DELAY_INIT |
+ USB_QUIRK_DELAY_CTRL_MSG },

/* Corsair Strafe RGB */
{ USB_DEVICE(0x1b1c, 0x1b20), .driver_info = USB_QUIRK_DELAY_INIT |

[PATCH resend] vfs: namei: use path_equal() in follow_dotdot()

[Danilo Krummrich]

Use path_equal() to detect whether we're already in root.

Signed-off-by: Danilo Krummrich <danilokrummr...@dk-develop.de>
---
 fs/namei.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index 186bd2464fd5..6f0dc40f88c5 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1438,10 +1438,8 @@ static int path_parent_directory(struct path *path)
 static int follow_dotdot(struct nameidata *nd)
 {
while(1) {
-   if (nd->path.dentry == nd->root.dentry &&
-   nd->path.mnt == nd->root.mnt) {
+   if (path_equal(>path, >root))
break;
-   }
if (nd->path.dentry != nd->path.mnt->mnt_root) {
int ret = path_parent_directory(>path);
if (ret)
-- 
2.17.0

[PATCH resend] vfs: namei: use path_equal() in follow_dotdot()

[Danilo Krummrich]

Use path_equal() to detect whether we're already in root.

Signed-off-by: Danilo Krummrich 
---
 fs/namei.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index 186bd2464fd5..6f0dc40f88c5 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1438,10 +1438,8 @@ static int path_parent_directory(struct path *path)
 static int follow_dotdot(struct nameidata *nd)
 {
while(1) {
-   if (nd->path.dentry == nd->root.dentry &&
-   nd->path.mnt == nd->root.mnt) {
+   if (path_equal(>path, >root))
break;
-   }
if (nd->path.dentry != nd->path.mnt->mnt_root) {
int ret = path_parent_directory(>path);
if (ret)
-- 
2.17.0

[PATCH resend] usb: quirks: add control message delay for 1b1c:1b20

[Danilo Krummrich]

Corsair Strafe RGB keyboard does not respond to usb control messages
sometimes and hence generates timeouts.

Commit de3af5bf259d ("usb: quirks: add delay init quirk for Corsair
Strafe RGB keyboard") tried to fix those timeouts by adding
USB_QUIRK_DELAY_INIT.

Unfortunately, even with this quirk timeouts of usb_control_msg()
can still be seen, but with a lower frequency (approx. 1 out of 15):

[   29.103520] usb 1-8: string descriptor 0 read error: -110
[   34.363097] usb 1-8: can't set config #1, error -110

Adding further delays to different locations where usb control
messages are issued just moves the timeouts to other locations,
e.g.:

[   35.400533] usbhid 1-8:1.0: can't add hid device: -110
[   35.401014] usbhid: probe of 1-8:1.0 failed with error -110

The only way to reliably avoid those issues is having a pause after
each usb control message. In approx. 200 boot cycles no more timeouts
were seen.

Addionaly, keep USB_QUIRK_DELAY_INIT as it turned out to be necessary
to have the delay in hub_port_connect() after hub_port_init().

The overall boot time seems not to be influenced by these additional
delays, even on fast machines and lightweight distributions.

Fixes: de3af5bf259d ("usb: quirks: add delay init quirk for Corsair Strafe RGB 
keyboard")
Cc: sta...@vger.kernel.org
Signed-off-by: Danilo Krummrich <danilokrummr...@dk-develop.de>
---
 drivers/usb/core/message.c | 4 
 drivers/usb/core/quirks.c  | 3 ++-
 include/linux/usb/quirks.h | 3 +++
 3 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
index c64cf6c4a83d..0c11d40a12bc 100644
--- a/drivers/usb/core/message.c
+++ b/drivers/usb/core/message.c
@@ -151,6 +151,10 @@ int usb_control_msg(struct usb_device *dev, unsigned int 
pipe, __u8 request,
 
ret = usb_internal_control_msg(dev, pipe, dr, data, size, timeout);
 
+   /* Linger a bit, prior to the next control message. */
+   if (dev->quirks & USB_QUIRK_DELAY_CTRL_MSG)
+   msleep(200);
+
kfree(dr);
 
return ret;
diff --git a/drivers/usb/core/quirks.c b/drivers/usb/core/quirks.c
index f4a548471f0f..54b019e267c5 100644
--- a/drivers/usb/core/quirks.c
+++ b/drivers/usb/core/quirks.c
@@ -230,7 +230,8 @@ static const struct usb_device_id usb_quirk_list[] = {
{ USB_DEVICE(0x1b1c, 0x1b13), .driver_info = USB_QUIRK_DELAY_INIT },
 
/* Corsair Strafe RGB */
-   { USB_DEVICE(0x1b1c, 0x1b20), .driver_info = USB_QUIRK_DELAY_INIT },
+   { USB_DEVICE(0x1b1c, 0x1b20), .driver_info = USB_QUIRK_DELAY_INIT |
+ USB_QUIRK_DELAY_CTRL_MSG },
 
/* Corsair K70 LUX */
{ USB_DEVICE(0x1b1c, 0x1b36), .driver_info = USB_QUIRK_DELAY_INIT },
diff --git a/include/linux/usb/quirks.h b/include/linux/usb/quirks.h
index f1fcec2fd5f8..b7a99ce56bc9 100644
--- a/include/linux/usb/quirks.h
+++ b/include/linux/usb/quirks.h
@@ -63,4 +63,7 @@
  */
 #define USB_QUIRK_DISCONNECT_SUSPEND   BIT(12)
 
+/* Device needs a pause after every control message. */
+#define USB_QUIRK_DELAY_CTRL_MSG   BIT(13)
+
 #endif /* __LINUX_USB_QUIRKS_H */
-- 
2.16.2

[PATCH resend] usb: quirks: add control message delay for 1b1c:1b20

[Danilo Krummrich]

Corsair Strafe RGB keyboard does not respond to usb control messages
sometimes and hence generates timeouts.

Commit de3af5bf259d ("usb: quirks: add delay init quirk for Corsair
Strafe RGB keyboard") tried to fix those timeouts by adding
USB_QUIRK_DELAY_INIT.

Unfortunately, even with this quirk timeouts of usb_control_msg()
can still be seen, but with a lower frequency (approx. 1 out of 15):

[   29.103520] usb 1-8: string descriptor 0 read error: -110
[   34.363097] usb 1-8: can't set config #1, error -110

Adding further delays to different locations where usb control
messages are issued just moves the timeouts to other locations,
e.g.:

[   35.400533] usbhid 1-8:1.0: can't add hid device: -110
[   35.401014] usbhid: probe of 1-8:1.0 failed with error -110

The only way to reliably avoid those issues is having a pause after
each usb control message. In approx. 200 boot cycles no more timeouts
were seen.

Addionaly, keep USB_QUIRK_DELAY_INIT as it turned out to be necessary
to have the delay in hub_port_connect() after hub_port_init().

The overall boot time seems not to be influenced by these additional
delays, even on fast machines and lightweight distributions.

Fixes: de3af5bf259d ("usb: quirks: add delay init quirk for Corsair Strafe RGB 
keyboard")
Cc: sta...@vger.kernel.org
Signed-off-by: Danilo Krummrich 
---
 drivers/usb/core/message.c | 4 
 drivers/usb/core/quirks.c  | 3 ++-
 include/linux/usb/quirks.h | 3 +++
 3 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
index c64cf6c4a83d..0c11d40a12bc 100644
--- a/drivers/usb/core/message.c
+++ b/drivers/usb/core/message.c
@@ -151,6 +151,10 @@ int usb_control_msg(struct usb_device *dev, unsigned int 
pipe, __u8 request,
 
ret = usb_internal_control_msg(dev, pipe, dr, data, size, timeout);
 
+   /* Linger a bit, prior to the next control message. */
+   if (dev->quirks & USB_QUIRK_DELAY_CTRL_MSG)
+   msleep(200);
+
kfree(dr);
 
return ret;
diff --git a/drivers/usb/core/quirks.c b/drivers/usb/core/quirks.c
index f4a548471f0f..54b019e267c5 100644
--- a/drivers/usb/core/quirks.c
+++ b/drivers/usb/core/quirks.c
@@ -230,7 +230,8 @@ static const struct usb_device_id usb_quirk_list[] = {
{ USB_DEVICE(0x1b1c, 0x1b13), .driver_info = USB_QUIRK_DELAY_INIT },
 
/* Corsair Strafe RGB */
-   { USB_DEVICE(0x1b1c, 0x1b20), .driver_info = USB_QUIRK_DELAY_INIT },
+   { USB_DEVICE(0x1b1c, 0x1b20), .driver_info = USB_QUIRK_DELAY_INIT |
+ USB_QUIRK_DELAY_CTRL_MSG },
 
/* Corsair K70 LUX */
{ USB_DEVICE(0x1b1c, 0x1b36), .driver_info = USB_QUIRK_DELAY_INIT },
diff --git a/include/linux/usb/quirks.h b/include/linux/usb/quirks.h
index f1fcec2fd5f8..b7a99ce56bc9 100644
--- a/include/linux/usb/quirks.h
+++ b/include/linux/usb/quirks.h
@@ -63,4 +63,7 @@
  */
 #define USB_QUIRK_DISCONNECT_SUSPEND   BIT(12)
 
+/* Device needs a pause after every control message. */
+#define USB_QUIRK_DELAY_CTRL_MSG   BIT(13)
+
 #endif /* __LINUX_USB_QUIRKS_H */
-- 
2.16.2

[PATCH v3 2/2] fs/sysctl: remove redundant link check in proc_sys_link_fill_cache()

[Danilo Krummrich]

proc_sys_link_fill_cache() does not need to check whether we're
called for a link - it's already done by scan().

Acked-by: Kees Cook <keesc...@chromium.org>
Signed-off-by: Danilo Krummrich <danilokrummr...@dk-develop.de>
---
v2: removed 'err' as it's only used for sysctl_follow_link()
v3: add ack
---
 fs/proc/proc_sysctl.c | 9 +++--
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
index 82ac5f682b73..d36ef667c0a8 100644
--- a/fs/proc/proc_sysctl.c
+++ b/fs/proc/proc_sysctl.c
@@ -712,12 +712,9 @@ static bool proc_sys_link_fill_cache(struct file *file,
if (IS_ERR(head))
return false;
 
-   if (S_ISLNK(table->mode)) {
-   /* It is not an error if we can not follow the link ignore it */
-   int err = sysctl_follow_link(, );
-   if (err)
-   goto out;
-   }
+   /* It is not an error if we can not follow the link ignore it */
+   if (sysctl_follow_link(, ))
+   goto out;
 
ret = proc_sys_fill_cache(file, ctx, head, table);
 out:
-- 
2.14.1

[PATCH v3 2/2] fs/sysctl: remove redundant link check in proc_sys_link_fill_cache()

[Danilo Krummrich]

proc_sys_link_fill_cache() does not need to check whether we're
called for a link - it's already done by scan().

Acked-by: Kees Cook 
Signed-off-by: Danilo Krummrich 
---
v2: removed 'err' as it's only used for sysctl_follow_link()
v3: add ack
---
 fs/proc/proc_sysctl.c | 9 +++--
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
index 82ac5f682b73..d36ef667c0a8 100644
--- a/fs/proc/proc_sysctl.c
+++ b/fs/proc/proc_sysctl.c
@@ -712,12 +712,9 @@ static bool proc_sys_link_fill_cache(struct file *file,
if (IS_ERR(head))
return false;
 
-   if (S_ISLNK(table->mode)) {
-   /* It is not an error if we can not follow the link ignore it */
-   int err = sysctl_follow_link(, );
-   if (err)
-   goto out;
-   }
+   /* It is not an error if we can not follow the link ignore it */
+   if (sysctl_follow_link(, ))
+   goto out;
 
ret = proc_sys_fill_cache(file, ctx, head, table);
 out:
-- 
2.14.1

[PATCH v3 1/2] fs/sysctl: fix potential page fault while unregistering sysctl table

[Danilo Krummrich]

proc_sys_link_fill_cache() does not take currently unregistering
sysctl tables into account, which might result into a page fault in
sysctl_follow_link() - add a check to fix it.

This bug has been present since v3.4.

Fixes: 0e47c99d7fe25 ("sysctl: Replace root_list with links between 
sysctl_table_sets")
Cc: sta...@vger.kernel.org
Acked-by: Kees Cook <keesc...@chromium.org>
Signed-off-by: Danilo Krummrich <danilokrummr...@dk-develop.de>
---
v2: removed empty line between between sysctl_head_grab and IS_ERR
v3: extend commit message, add tags and cc stable
---
 fs/proc/proc_sysctl.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
index c5cbbdff3c3d..82ac5f682b73 100644
--- a/fs/proc/proc_sysctl.c
+++ b/fs/proc/proc_sysctl.c
@@ -707,7 +707,10 @@ static bool proc_sys_link_fill_cache(struct file *file,
struct ctl_table *table)
 {
bool ret = true;
+
head = sysctl_head_grab(head);
+   if (IS_ERR(head))
+   return false;
 
if (S_ISLNK(table->mode)) {
/* It is not an error if we can not follow the link ignore it */
-- 
2.14.1

[PATCH v3 1/2] fs/sysctl: fix potential page fault while unregistering sysctl table

[Danilo Krummrich]

proc_sys_link_fill_cache() does not take currently unregistering
sysctl tables into account, which might result into a page fault in
sysctl_follow_link() - add a check to fix it.

This bug has been present since v3.4.

Fixes: 0e47c99d7fe25 ("sysctl: Replace root_list with links between 
sysctl_table_sets")
Cc: sta...@vger.kernel.org
Acked-by: Kees Cook 
Signed-off-by: Danilo Krummrich 
---
v2: removed empty line between between sysctl_head_grab and IS_ERR
v3: extend commit message, add tags and cc stable
---
 fs/proc/proc_sysctl.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
index c5cbbdff3c3d..82ac5f682b73 100644
--- a/fs/proc/proc_sysctl.c
+++ b/fs/proc/proc_sysctl.c
@@ -707,7 +707,10 @@ static bool proc_sys_link_fill_cache(struct file *file,
struct ctl_table *table)
 {
bool ret = true;
+
head = sysctl_head_grab(head);
+   if (IS_ERR(head))
+   return false;
 
if (S_ISLNK(table->mode)) {
/* It is not an error if we can not follow the link ignore it */
-- 
2.14.1

[PATCH v2 1/2] fs/sysctl: fix potential page fault while unregistering sysctl table

[Danilo Krummrich]

proc_sys_link_fill_cache() does not take currently unregistering
sysctl tables into account, which might result into a page fault in
sysctl_follow_link() - add a check to fix it.

Signed-off-by: Danilo Krummrich <danilokrummr...@dk-develop.de>
---
v2: removed empty line between between sysctl_head_grab and IS_ERR
---
 fs/proc/proc_sysctl.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
index c5cbbdff3c3d..82ac5f682b73 100644
--- a/fs/proc/proc_sysctl.c
+++ b/fs/proc/proc_sysctl.c
@@ -707,7 +707,10 @@ static bool proc_sys_link_fill_cache(struct file *file,
struct ctl_table *table)
 {
bool ret = true;
+
head = sysctl_head_grab(head);
+   if (IS_ERR(head))
+   return false;
 
if (S_ISLNK(table->mode)) {
/* It is not an error if we can not follow the link ignore it */
-- 
2.14.1

[PATCH v2 1/2] fs/sysctl: fix potential page fault while unregistering sysctl table

[Danilo Krummrich]

proc_sys_link_fill_cache() does not take currently unregistering
sysctl tables into account, which might result into a page fault in
sysctl_follow_link() - add a check to fix it.

Signed-off-by: Danilo Krummrich 
---
v2: removed empty line between between sysctl_head_grab and IS_ERR
---
 fs/proc/proc_sysctl.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
index c5cbbdff3c3d..82ac5f682b73 100644
--- a/fs/proc/proc_sysctl.c
+++ b/fs/proc/proc_sysctl.c
@@ -707,7 +707,10 @@ static bool proc_sys_link_fill_cache(struct file *file,
struct ctl_table *table)
 {
bool ret = true;
+
head = sysctl_head_grab(head);
+   if (IS_ERR(head))
+   return false;
 
if (S_ISLNK(table->mode)) {
/* It is not an error if we can not follow the link ignore it */
-- 
2.14.1

[PATCH v2 2/2] fs/sysctl: remove redundant link check in proc_sys_link_fill_cache()

[Danilo Krummrich]

proc_sys_link_fill_cache() does not need to check whether we're
called for a link - it's already done by scan().

Signed-off-by: Danilo Krummrich <danilokrummr...@dk-develop.de>
---
v2: removed 'err' as it's only used for sysctl_follow_link()
---
 fs/proc/proc_sysctl.c | 9 +++--
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
index 82ac5f682b73..d36ef667c0a8 100644
--- a/fs/proc/proc_sysctl.c
+++ b/fs/proc/proc_sysctl.c
@@ -712,12 +712,9 @@ static bool proc_sys_link_fill_cache(struct file *file,
if (IS_ERR(head))
return false;
 
-   if (S_ISLNK(table->mode)) {
-   /* It is not an error if we can not follow the link ignore it */
-   int err = sysctl_follow_link(, );
-   if (err)
-   goto out;
-   }
+   /* It is not an error if we can not follow the link ignore it */
+   if (sysctl_follow_link(, ))
+   goto out;
 
ret = proc_sys_fill_cache(file, ctx, head, table);
 out:
-- 
2.14.1

[PATCH v2 2/2] fs/sysctl: remove redundant link check in proc_sys_link_fill_cache()

[Danilo Krummrich]

proc_sys_link_fill_cache() does not need to check whether we're
called for a link - it's already done by scan().

Signed-off-by: Danilo Krummrich 
---
v2: removed 'err' as it's only used for sysctl_follow_link()
---
 fs/proc/proc_sysctl.c | 9 +++--
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
index 82ac5f682b73..d36ef667c0a8 100644
--- a/fs/proc/proc_sysctl.c
+++ b/fs/proc/proc_sysctl.c
@@ -712,12 +712,9 @@ static bool proc_sys_link_fill_cache(struct file *file,
if (IS_ERR(head))
return false;
 
-   if (S_ISLNK(table->mode)) {
-   /* It is not an error if we can not follow the link ignore it */
-   int err = sysctl_follow_link(, );
-   if (err)
-   goto out;
-   }
+   /* It is not an error if we can not follow the link ignore it */
+   if (sysctl_follow_link(, ))
+   goto out;
 
ret = proc_sys_fill_cache(file, ctx, head, table);
 out:
-- 
2.14.1

Re: [PATCH 1/2] fs/sysctl: fix potential page fault while unregistering sysctl table

[Danilo Krummrich]

On 2018-02-28 00:02, Kees Cook wrote:

On Tue, Feb 27, 2018 at 2:43 PM, Danilo Krummrich
<danilokrummr...@dk-develop.de> wrote:

proc_sys_link_fill_cache() does not take currently unregistering
sysctl tables into account, which might result into a page fault in
sysctl_follow_link() - add a check to fix it.

Signed-off-by: Danilo Krummrich <danilokrummr...@dk-develop.de>
---
 fs/proc/proc_sysctl.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
index c5cbbdff3c3d..a0b6c647835e 100644
--- a/fs/proc/proc_sysctl.c
+++ b/fs/proc/proc_sysctl.c
@@ -709,6 +709,9 @@ static bool proc_sys_link_fill_cache(struct file 
*file,

bool ret = true;


Nothing appears to actually change "ret" in this function. It should
likely be dropped too.


proc_sys_fill_cache() potentially changes "ret".

head = sysctl_head_grab(head);

+   if (IS_ERR(head))
+   return false;
+


This looks sensible. I'd drop the blank line between sysctl_head_grab
and the IS_ERR, though.


I'll do that.

How are you testing this change?


Honestly, not at all. Actually, I never run in such a page fault.
I spotted it by accident while reading the code.

Thanks!

-Kees


if (S_ISLNK(table->mode)) {
/* It is not an error if we can not follow the link 
ignore it */

int err = sysctl_follow_link(, );
--
2.14.1

Re: [PATCH 1/2] fs/sysctl: fix potential page fault while unregistering sysctl table

[Danilo Krummrich]

On 2018-02-28 00:02, Kees Cook wrote:

On Tue, Feb 27, 2018 at 2:43 PM, Danilo Krummrich
 wrote:

proc_sys_link_fill_cache() does not take currently unregistering
sysctl tables into account, which might result into a page fault in
sysctl_follow_link() - add a check to fix it.

Signed-off-by: Danilo Krummrich 
---
 fs/proc/proc_sysctl.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
index c5cbbdff3c3d..a0b6c647835e 100644
--- a/fs/proc/proc_sysctl.c
+++ b/fs/proc/proc_sysctl.c
@@ -709,6 +709,9 @@ static bool proc_sys_link_fill_cache(struct file 
*file,

bool ret = true;


Nothing appears to actually change "ret" in this function. It should
likely be dropped too.


proc_sys_fill_cache() potentially changes "ret".

head = sysctl_head_grab(head);

+   if (IS_ERR(head))
+   return false;
+


This looks sensible. I'd drop the blank line between sysctl_head_grab
and the IS_ERR, though.


I'll do that.

How are you testing this change?


Honestly, not at all. Actually, I never run in such a page fault.
I spotted it by accident while reading the code.

Thanks!

-Kees


if (S_ISLNK(table->mode)) {
/* It is not an error if we can not follow the link 
ignore it */

int err = sysctl_follow_link(, );
--
2.14.1

Re: [PATCH 2/2] fs/sysctl: remove redundant link check in proc_sys_link_fill_cache()

[Danilo Krummrich]

On 2018-02-27 23:59, Kees Cook wrote:

On Tue, Feb 27, 2018 at 2:43 PM, Danilo Krummrich
<danilokrummr...@dk-develop.de> wrote:

proc_sys_link_fill_cache() does not need to check whether we're
called for a link - it's already done by scan().

Signed-off-by: Danilo Krummrich <danilokrummr...@dk-develop.de>
---
 fs/proc/proc_sysctl.c | 11 +--
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
index a0b6c647835e..7e7d9facb842 100644
--- a/fs/proc/proc_sysctl.c
+++ b/fs/proc/proc_sysctl.c
@@ -707,17 +707,16 @@ static bool proc_sys_link_fill_cache(struct file 
*file,

struct ctl_table *table)
 {
bool ret = true;
+   int err = 0;
head = sysctl_head_grab(head);

if (IS_ERR(head))
return false;

-   if (S_ISLNK(table->mode)) {
-   /* It is not an error if we can not follow the link 
ignore it */

-   int err = sysctl_follow_link(, );
-   if (err)
-   goto out;
-   }
+   /* It is not an error if we can not follow the link ignore it 
*/

+   sysctl_follow_link(, );


Shouldn't this be err = sysctl_follow_link... ? Otherwise I don't see
where err is used.

-Kees


Of course, thanks.

+   if (err)
+   goto out;

ret = proc_sys_fill_cache(file, ctx, head, table);
 out:
--
2.14.1

Re: [PATCH 2/2] fs/sysctl: remove redundant link check in proc_sys_link_fill_cache()

[Danilo Krummrich]

On 2018-02-27 23:59, Kees Cook wrote:

On Tue, Feb 27, 2018 at 2:43 PM, Danilo Krummrich
 wrote:

proc_sys_link_fill_cache() does not need to check whether we're
called for a link - it's already done by scan().

Signed-off-by: Danilo Krummrich 
---
 fs/proc/proc_sysctl.c | 11 +--
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
index a0b6c647835e..7e7d9facb842 100644
--- a/fs/proc/proc_sysctl.c
+++ b/fs/proc/proc_sysctl.c
@@ -707,17 +707,16 @@ static bool proc_sys_link_fill_cache(struct file 
*file,

struct ctl_table *table)
 {
bool ret = true;
+   int err = 0;
head = sysctl_head_grab(head);

if (IS_ERR(head))
return false;

-   if (S_ISLNK(table->mode)) {
-   /* It is not an error if we can not follow the link 
ignore it */

-   int err = sysctl_follow_link(, );
-   if (err)
-   goto out;
-   }
+   /* It is not an error if we can not follow the link ignore it 
*/

+   sysctl_follow_link(, );


Shouldn't this be err = sysctl_follow_link... ? Otherwise I don't see
where err is used.

-Kees


Of course, thanks.

+   if (err)
+   goto out;

ret = proc_sys_fill_cache(file, ctx, head, table);
 out:
--
2.14.1

[PATCH 1/2] fs/sysctl: fix potential page fault while unregistering sysctl table

[Danilo Krummrich]

proc_sys_link_fill_cache() does not take currently unregistering
sysctl tables into account, which might result into a page fault in
sysctl_follow_link() - add a check to fix it.

Signed-off-by: Danilo Krummrich <danilokrummr...@dk-develop.de>
---
 fs/proc/proc_sysctl.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
index c5cbbdff3c3d..a0b6c647835e 100644
--- a/fs/proc/proc_sysctl.c
+++ b/fs/proc/proc_sysctl.c
@@ -709,6 +709,9 @@ static bool proc_sys_link_fill_cache(struct file *file,
bool ret = true;
head = sysctl_head_grab(head);
 
+   if (IS_ERR(head))
+   return false;
+
if (S_ISLNK(table->mode)) {
/* It is not an error if we can not follow the link ignore it */
int err = sysctl_follow_link(, );
-- 
2.14.1

[PATCH 1/2] fs/sysctl: fix potential page fault while unregistering sysctl table

[Danilo Krummrich]

proc_sys_link_fill_cache() does not take currently unregistering
sysctl tables into account, which might result into a page fault in
sysctl_follow_link() - add a check to fix it.

Signed-off-by: Danilo Krummrich 
---
 fs/proc/proc_sysctl.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
index c5cbbdff3c3d..a0b6c647835e 100644
--- a/fs/proc/proc_sysctl.c
+++ b/fs/proc/proc_sysctl.c
@@ -709,6 +709,9 @@ static bool proc_sys_link_fill_cache(struct file *file,
bool ret = true;
head = sysctl_head_grab(head);
 
+   if (IS_ERR(head))
+   return false;
+
if (S_ISLNK(table->mode)) {
/* It is not an error if we can not follow the link ignore it */
int err = sysctl_follow_link(, );
-- 
2.14.1

[PATCH 2/2] fs/sysctl: remove redundant link check in proc_sys_link_fill_cache()

[Danilo Krummrich]

proc_sys_link_fill_cache() does not need to check whether we're
called for a link - it's already done by scan().

Signed-off-by: Danilo Krummrich <danilokrummr...@dk-develop.de>
---
 fs/proc/proc_sysctl.c | 11 +--
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
index a0b6c647835e..7e7d9facb842 100644
--- a/fs/proc/proc_sysctl.c
+++ b/fs/proc/proc_sysctl.c
@@ -707,17 +707,16 @@ static bool proc_sys_link_fill_cache(struct file *file,
struct ctl_table *table)
 {
bool ret = true;
+   int err = 0;
head = sysctl_head_grab(head);
 
if (IS_ERR(head))
return false;
 
-   if (S_ISLNK(table->mode)) {
-   /* It is not an error if we can not follow the link ignore it */
-   int err = sysctl_follow_link(, );
-   if (err)
-   goto out;
-   }
+   /* It is not an error if we can not follow the link ignore it */
+   sysctl_follow_link(, );
+   if (err)
+   goto out;
 
ret = proc_sys_fill_cache(file, ctx, head, table);
 out:
-- 
2.14.1

[PATCH 2/2] fs/sysctl: remove redundant link check in proc_sys_link_fill_cache()

[Danilo Krummrich]

proc_sys_link_fill_cache() does not need to check whether we're
called for a link - it's already done by scan().

Signed-off-by: Danilo Krummrich 
---
 fs/proc/proc_sysctl.c | 11 +--
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
index a0b6c647835e..7e7d9facb842 100644
--- a/fs/proc/proc_sysctl.c
+++ b/fs/proc/proc_sysctl.c
@@ -707,17 +707,16 @@ static bool proc_sys_link_fill_cache(struct file *file,
struct ctl_table *table)
 {
bool ret = true;
+   int err = 0;
head = sysctl_head_grab(head);
 
if (IS_ERR(head))
return false;
 
-   if (S_ISLNK(table->mode)) {
-   /* It is not an error if we can not follow the link ignore it */
-   int err = sysctl_follow_link(, );
-   if (err)
-   goto out;
-   }
+   /* It is not an error if we can not follow the link ignore it */
+   sysctl_follow_link(, );
+   if (err)
+   goto out;
 
ret = proc_sys_fill_cache(file, ctx, head, table);
 out:
-- 
2.14.1

[PATCH] usb: quirks: add control message delay for 1b1c:1b20

[Danilo Krummrich]

Corsair Strafe RGB keyboard does not respond to usb control messages
sometimes and hence generates timeouts.

Commit de3af5bf259d ("usb: quirks: add delay init quirk for Corsair
Strafe RGB keyboard") tried to fix those timeouts by adding
USB_QUIRK_DELAY_INIT.

Unfortunately, even with this quirk timeouts of usb_control_msg()
can still be seen, but with a lower frequency (approx. 1 out of 15):

[   29.103520] usb 1-8: string descriptor 0 read error: -110
[   34.363097] usb 1-8: can't set config #1, error -110

Adding further delays to different locations where usb control
messages are issued just moves the timeouts to other locations,
e.g.:

[   35.400533] usbhid 1-8:1.0: can't add hid device: -110
[   35.401014] usbhid: probe of 1-8:1.0 failed with error -110

The only way to reliably avoid those issues is having a pause after
each usb control message. In approx. 200 boot cycles no more timeouts
were seen.

Addionaly, keep USB_QUIRK_DELAY_INIT as it turned out to be necessary
to have the delay in hub_port_connect() after hub_port_init().

The overall boot time seems not to be influenced by these additional
delays, even on fast machines.

Signed-off-by: Danilo Krummrich <danilokrummr...@dk-develop.de>
---
 drivers/usb/core/message.c | 5 +
 drivers/usb/core/quirks.c  | 3 ++-
 include/linux/usb/quirks.h | 3 +++
 3 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
index 77001bcfc504..cbab2a99ce16 100644
--- a/drivers/usb/core/message.c
+++ b/drivers/usb/core/message.c
@@ -150,6 +150,10 @@ int usb_control_msg(struct usb_device *dev, unsigned int 
pipe, __u8 request,
 
ret = usb_internal_control_msg(dev, pipe, dr, data, size, timeout);
 
+   /* Linger a bit, prior to the next control message. */
+   if (dev->quirks & USB_QUIRK_DELAY_CTRL_MSG)
+   msleep(200);
+
kfree(dr);
 
return ret;
@@ -642,6 +646,7 @@ int usb_get_descriptor(struct usb_device *dev, unsigned 
char type,
USB_REQ_GET_DESCRIPTOR, USB_DIR_IN,
(type << 8) + index, 0, buf, size,
USB_CTRL_GET_TIMEOUT);
+
if (result <= 0 && result != -ETIMEDOUT)
continue;
if (result > 1 && ((u8 *)buf)[1] != type) {
diff --git a/drivers/usb/core/quirks.c b/drivers/usb/core/quirks.c
index 4024926c1d68..4450bec7f3af 100644
--- a/drivers/usb/core/quirks.c
+++ b/drivers/usb/core/quirks.c
@@ -227,7 +227,8 @@ static const struct usb_device_id usb_quirk_list[] = {
USB_QUIRK_LINEAR_UFRAME_INTR_BINTERVAL },
 
/* Corsair Strafe RGB */
-   { USB_DEVICE(0x1b1c, 0x1b20), .driver_info = USB_QUIRK_DELAY_INIT },
+   { USB_DEVICE(0x1b1c, 0x1b20), .driver_info = USB_QUIRK_DELAY_INIT |
+ USB_QUIRK_DELAY_CTRL_MSG },
 
/* Corsair K70 LUX */
{ USB_DEVICE(0x1b1c, 0x1b36), .driver_info = USB_QUIRK_DELAY_INIT },
diff --git a/include/linux/usb/quirks.h b/include/linux/usb/quirks.h
index f1fcec2fd5f8..b7a99ce56bc9 100644
--- a/include/linux/usb/quirks.h
+++ b/include/linux/usb/quirks.h
@@ -63,4 +63,7 @@
  */
 #define USB_QUIRK_DISCONNECT_SUSPEND   BIT(12)
 
+/* Device needs a pause after every control message. */
+#define USB_QUIRK_DELAY_CTRL_MSG   BIT(13)
+
 #endif /* __LINUX_USB_QUIRKS_H */
-- 
2.14.1

[PATCH] usb: quirks: add control message delay for 1b1c:1b20

[Danilo Krummrich]

Corsair Strafe RGB keyboard does not respond to usb control messages
sometimes and hence generates timeouts.

Commit de3af5bf259d ("usb: quirks: add delay init quirk for Corsair
Strafe RGB keyboard") tried to fix those timeouts by adding
USB_QUIRK_DELAY_INIT.

Unfortunately, even with this quirk timeouts of usb_control_msg()
can still be seen, but with a lower frequency (approx. 1 out of 15):

[   29.103520] usb 1-8: string descriptor 0 read error: -110
[   34.363097] usb 1-8: can't set config #1, error -110

Adding further delays to different locations where usb control
messages are issued just moves the timeouts to other locations,
e.g.:

[   35.400533] usbhid 1-8:1.0: can't add hid device: -110
[   35.401014] usbhid: probe of 1-8:1.0 failed with error -110

The only way to reliably avoid those issues is having a pause after
each usb control message. In approx. 200 boot cycles no more timeouts
were seen.

Addionaly, keep USB_QUIRK_DELAY_INIT as it turned out to be necessary
to have the delay in hub_port_connect() after hub_port_init().

The overall boot time seems not to be influenced by these additional
delays, even on fast machines.

Signed-off-by: Danilo Krummrich 
---
 drivers/usb/core/message.c | 5 +
 drivers/usb/core/quirks.c  | 3 ++-
 include/linux/usb/quirks.h | 3 +++
 3 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
index 77001bcfc504..cbab2a99ce16 100644
--- a/drivers/usb/core/message.c
+++ b/drivers/usb/core/message.c
@@ -150,6 +150,10 @@ int usb_control_msg(struct usb_device *dev, unsigned int 
pipe, __u8 request,
 
ret = usb_internal_control_msg(dev, pipe, dr, data, size, timeout);
 
+   /* Linger a bit, prior to the next control message. */
+   if (dev->quirks & USB_QUIRK_DELAY_CTRL_MSG)
+   msleep(200);
+
kfree(dr);
 
return ret;
@@ -642,6 +646,7 @@ int usb_get_descriptor(struct usb_device *dev, unsigned 
char type,
USB_REQ_GET_DESCRIPTOR, USB_DIR_IN,
(type << 8) + index, 0, buf, size,
USB_CTRL_GET_TIMEOUT);
+
if (result <= 0 && result != -ETIMEDOUT)
continue;
if (result > 1 && ((u8 *)buf)[1] != type) {
diff --git a/drivers/usb/core/quirks.c b/drivers/usb/core/quirks.c
index 4024926c1d68..4450bec7f3af 100644
--- a/drivers/usb/core/quirks.c
+++ b/drivers/usb/core/quirks.c
@@ -227,7 +227,8 @@ static const struct usb_device_id usb_quirk_list[] = {
USB_QUIRK_LINEAR_UFRAME_INTR_BINTERVAL },
 
/* Corsair Strafe RGB */
-   { USB_DEVICE(0x1b1c, 0x1b20), .driver_info = USB_QUIRK_DELAY_INIT },
+   { USB_DEVICE(0x1b1c, 0x1b20), .driver_info = USB_QUIRK_DELAY_INIT |
+ USB_QUIRK_DELAY_CTRL_MSG },
 
/* Corsair K70 LUX */
{ USB_DEVICE(0x1b1c, 0x1b36), .driver_info = USB_QUIRK_DELAY_INIT },
diff --git a/include/linux/usb/quirks.h b/include/linux/usb/quirks.h
index f1fcec2fd5f8..b7a99ce56bc9 100644
--- a/include/linux/usb/quirks.h
+++ b/include/linux/usb/quirks.h
@@ -63,4 +63,7 @@
  */
 #define USB_QUIRK_DISCONNECT_SUSPEND   BIT(12)
 
+/* Device needs a pause after every control message. */
+#define USB_QUIRK_DELAY_CTRL_MSG   BIT(13)
+
 #endif /* __LINUX_USB_QUIRKS_H */
-- 
2.14.1

[PATCH v2] usb: quirks: add control message delay for 1b1c:1b20

[Danilo Krummrich]

Corsair Strafe RGB keyboard does not respond to usb control messages
sometimes and hence generates timeouts.

Commit de3af5bf259d ("usb: quirks: add delay init quirk for Corsair
Strafe RGB keyboard") tried to fix those timeouts by adding
USB_QUIRK_DELAY_INIT.

Unfortunately, even with this quirk timeouts of usb_control_msg()
can still be seen, but with a lower frequency (approx. 1 out of 15):

[   29.103520] usb 1-8: string descriptor 0 read error: -110
[   34.363097] usb 1-8: can't set config #1, error -110

Adding further delays to different locations where usb control
messages are issued just moves the timeouts to other locations,
e.g.:

[   35.400533] usbhid 1-8:1.0: can't add hid device: -110
[   35.401014] usbhid: probe of 1-8:1.0 failed with error -110

The only way to reliably avoid those issues is having a pause after
each usb control message. In approx. 200 boot cycles no more timeouts
were seen.

Addionaly, keep USB_QUIRK_DELAY_INIT as it turned out to be necessary
to have the delay in hub_port_connect() after hub_port_init().

The overall boot time seems not to be influenced by these additional
delays, even on fast machines.

Signed-off-by: Danilo Krummrich <danilokrummr...@dk-develop.de>
---
v2: remove empty line in usb_get_descriptor() added by accident
---
 drivers/usb/core/message.c | 4 
 drivers/usb/core/quirks.c  | 3 ++-
 include/linux/usb/quirks.h | 3 +++
 3 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
index 77001bcfc504..a4025760dd84 100644
--- a/drivers/usb/core/message.c
+++ b/drivers/usb/core/message.c
@@ -150,6 +150,10 @@ int usb_control_msg(struct usb_device *dev, unsigned int 
pipe, __u8 request,
 
ret = usb_internal_control_msg(dev, pipe, dr, data, size, timeout);
 
+   /* Linger a bit, prior to the next control message. */
+   if (dev->quirks & USB_QUIRK_DELAY_CTRL_MSG)
+   msleep(200);
+
kfree(dr);
 
return ret;
diff --git a/drivers/usb/core/quirks.c b/drivers/usb/core/quirks.c
index 4024926c1d68..4450bec7f3af 100644
--- a/drivers/usb/core/quirks.c
+++ b/drivers/usb/core/quirks.c
@@ -227,7 +227,8 @@ static const struct usb_device_id usb_quirk_list[] = {
USB_QUIRK_LINEAR_UFRAME_INTR_BINTERVAL },
 
/* Corsair Strafe RGB */
-   { USB_DEVICE(0x1b1c, 0x1b20), .driver_info = USB_QUIRK_DELAY_INIT },
+   { USB_DEVICE(0x1b1c, 0x1b20), .driver_info = USB_QUIRK_DELAY_INIT |
+ USB_QUIRK_DELAY_CTRL_MSG },
 
/* Corsair K70 LUX */
{ USB_DEVICE(0x1b1c, 0x1b36), .driver_info = USB_QUIRK_DELAY_INIT },
diff --git a/include/linux/usb/quirks.h b/include/linux/usb/quirks.h
index f1fcec2fd5f8..b7a99ce56bc9 100644
--- a/include/linux/usb/quirks.h
+++ b/include/linux/usb/quirks.h
@@ -63,4 +63,7 @@
  */
 #define USB_QUIRK_DISCONNECT_SUSPEND   BIT(12)
 
+/* Device needs a pause after every control message. */
+#define USB_QUIRK_DELAY_CTRL_MSG   BIT(13)
+
 #endif /* __LINUX_USB_QUIRKS_H */
-- 
2.14.1

[PATCH v2] usb: quirks: add control message delay for 1b1c:1b20

[Danilo Krummrich]

Corsair Strafe RGB keyboard does not respond to usb control messages
sometimes and hence generates timeouts.

Commit de3af5bf259d ("usb: quirks: add delay init quirk for Corsair
Strafe RGB keyboard") tried to fix those timeouts by adding
USB_QUIRK_DELAY_INIT.

Unfortunately, even with this quirk timeouts of usb_control_msg()
can still be seen, but with a lower frequency (approx. 1 out of 15):

[   29.103520] usb 1-8: string descriptor 0 read error: -110
[   34.363097] usb 1-8: can't set config #1, error -110

Adding further delays to different locations where usb control
messages are issued just moves the timeouts to other locations,
e.g.:

[   35.400533] usbhid 1-8:1.0: can't add hid device: -110
[   35.401014] usbhid: probe of 1-8:1.0 failed with error -110

The only way to reliably avoid those issues is having a pause after
each usb control message. In approx. 200 boot cycles no more timeouts
were seen.

Addionaly, keep USB_QUIRK_DELAY_INIT as it turned out to be necessary
to have the delay in hub_port_connect() after hub_port_init().

The overall boot time seems not to be influenced by these additional
delays, even on fast machines.

Signed-off-by: Danilo Krummrich 
---
v2: remove empty line in usb_get_descriptor() added by accident
---
 drivers/usb/core/message.c | 4 
 drivers/usb/core/quirks.c  | 3 ++-
 include/linux/usb/quirks.h | 3 +++
 3 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
index 77001bcfc504..a4025760dd84 100644
--- a/drivers/usb/core/message.c
+++ b/drivers/usb/core/message.c
@@ -150,6 +150,10 @@ int usb_control_msg(struct usb_device *dev, unsigned int 
pipe, __u8 request,
 
ret = usb_internal_control_msg(dev, pipe, dr, data, size, timeout);
 
+   /* Linger a bit, prior to the next control message. */
+   if (dev->quirks & USB_QUIRK_DELAY_CTRL_MSG)
+   msleep(200);
+
kfree(dr);
 
return ret;
diff --git a/drivers/usb/core/quirks.c b/drivers/usb/core/quirks.c
index 4024926c1d68..4450bec7f3af 100644
--- a/drivers/usb/core/quirks.c
+++ b/drivers/usb/core/quirks.c
@@ -227,7 +227,8 @@ static const struct usb_device_id usb_quirk_list[] = {
USB_QUIRK_LINEAR_UFRAME_INTR_BINTERVAL },
 
/* Corsair Strafe RGB */
-   { USB_DEVICE(0x1b1c, 0x1b20), .driver_info = USB_QUIRK_DELAY_INIT },
+   { USB_DEVICE(0x1b1c, 0x1b20), .driver_info = USB_QUIRK_DELAY_INIT |
+ USB_QUIRK_DELAY_CTRL_MSG },
 
/* Corsair K70 LUX */
{ USB_DEVICE(0x1b1c, 0x1b36), .driver_info = USB_QUIRK_DELAY_INIT },
diff --git a/include/linux/usb/quirks.h b/include/linux/usb/quirks.h
index f1fcec2fd5f8..b7a99ce56bc9 100644
--- a/include/linux/usb/quirks.h
+++ b/include/linux/usb/quirks.h
@@ -63,4 +63,7 @@
  */
 #define USB_QUIRK_DISCONNECT_SUSPEND   BIT(12)
 
+/* Device needs a pause after every control message. */
+#define USB_QUIRK_DELAY_CTRL_MSG   BIT(13)
+
 #endif /* __LINUX_USB_QUIRKS_H */
-- 
2.14.1

Re: [PATCH] Input: ps2-gpio - actually abort probe when connected to sleeping GPIOs

[Danilo Krummrich]

On 2017-10-24 01:44, Dmitry Torokhov wrote:

We've been missing a goto to the unwind path...

Signed-off-by: Dmitry Torokhov 
---
 drivers/input/serio/ps2-gpio.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/input/serio/ps2-gpio.c 
b/drivers/input/serio/ps2-gpio.c

index b50e3817f3c4..c62cceb97bb1 100644
--- a/drivers/input/serio/ps2-gpio.c
+++ b/drivers/input/serio/ps2-gpio.c
@@ -366,6 +366,7 @@ static int ps2_gpio_probe(struct platform_device 
*pdev)

gpiod_cansleep(drvdata->gpio_clk)) {
dev_err(dev, "GPIO data or clk are connected via slow bus\n");
error = -EINVAL;
+   goto err_free_serio;

Good catch, thanks.

}

drvdata->irq = platform_get_irq(pdev, 0);
--
2.15.0.rc0.271.g36b669edcc-goog

Re: [PATCH] Input: ps2-gpio - actually abort probe when connected to sleeping GPIOs

[Danilo Krummrich]

On 2017-10-24 01:44, Dmitry Torokhov wrote:

We've been missing a goto to the unwind path...

Signed-off-by: Dmitry Torokhov 
---
 drivers/input/serio/ps2-gpio.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/input/serio/ps2-gpio.c 
b/drivers/input/serio/ps2-gpio.c

index b50e3817f3c4..c62cceb97bb1 100644
--- a/drivers/input/serio/ps2-gpio.c
+++ b/drivers/input/serio/ps2-gpio.c
@@ -366,6 +366,7 @@ static int ps2_gpio_probe(struct platform_device 
*pdev)

gpiod_cansleep(drvdata->gpio_clk)) {
dev_err(dev, "GPIO data or clk are connected via slow bus\n");
error = -EINVAL;
+   goto err_free_serio;

Good catch, thanks.

}

drvdata->irq = platform_get_irq(pdev, 0);
--
2.15.0.rc0.271.g36b669edcc-goog

[PATCH v9 2/2] dt-bindings: new binding for ps/2 gpio devices

[Danilo Krummrich]

The PS/2 gpio device binding defines the gpio pins (data and clock)
as well as the interrupt which should be used to drive the ps/2 bus.
It is expected to get an interrupt on the falling edge of the clock
line.

Also it can be configured whether the host should support writing to
the device.

Signed-off-by: Danilo Krummrich <danilokrummr...@dk-develop.de>
---
 .../devicetree/bindings/serio/ps2-gpio.txt | 23 ++
 1 file changed, 23 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/serio/ps2-gpio.txt

diff --git a/Documentation/devicetree/bindings/serio/ps2-gpio.txt 
b/Documentation/devicetree/bindings/serio/ps2-gpio.txt
new file mode 100644
index ..7b7bc9cdf986
--- /dev/null
+++ b/Documentation/devicetree/bindings/serio/ps2-gpio.txt
@@ -0,0 +1,23 @@
+Device-Tree binding for ps/2 gpio device
+
+Required properties:
+   - compatible = "ps2-gpio"
+   - data-gpios: the data pin
+   - clk-gpios: the clock pin
+   - interrupts: Should trigger on the falling edge of the clock line.
+
+Optional properties:
+   - write-enable: Indicates whether write function is provided
+   to serio device. Possibly providing the write fn will not work, because
+   of the tough timing requirements.
+
+Example nodes:
+
+ps2@0 {
+   compatible = "ps2-gpio";
+   interrupt-parent = <>;
+   interrupts = <23 IRQ_TYPE_EDGE_FALLING>;
+   data-gpios = < 24 GPIO_ACTIVE_HIGH>;
+   clk-gpios = < 23 GPIO_ACTIVE_HIGH>;
+   write-enable;
+};
-- 
2.14.1

[PATCH v9 2/2] dt-bindings: new binding for ps/2 gpio devices

[Danilo Krummrich]

The PS/2 gpio device binding defines the gpio pins (data and clock)
as well as the interrupt which should be used to drive the ps/2 bus.
It is expected to get an interrupt on the falling edge of the clock
line.

Also it can be configured whether the host should support writing to
the device.

Signed-off-by: Danilo Krummrich 
---
 .../devicetree/bindings/serio/ps2-gpio.txt | 23 ++
 1 file changed, 23 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/serio/ps2-gpio.txt

diff --git a/Documentation/devicetree/bindings/serio/ps2-gpio.txt 
b/Documentation/devicetree/bindings/serio/ps2-gpio.txt
new file mode 100644
index ..7b7bc9cdf986
--- /dev/null
+++ b/Documentation/devicetree/bindings/serio/ps2-gpio.txt
@@ -0,0 +1,23 @@
+Device-Tree binding for ps/2 gpio device
+
+Required properties:
+   - compatible = "ps2-gpio"
+   - data-gpios: the data pin
+   - clk-gpios: the clock pin
+   - interrupts: Should trigger on the falling edge of the clock line.
+
+Optional properties:
+   - write-enable: Indicates whether write function is provided
+   to serio device. Possibly providing the write fn will not work, because
+   of the tough timing requirements.
+
+Example nodes:
+
+ps2@0 {
+   compatible = "ps2-gpio";
+   interrupt-parent = <>;
+   interrupts = <23 IRQ_TYPE_EDGE_FALLING>;
+   data-gpios = < 24 GPIO_ACTIVE_HIGH>;
+   clk-gpios = < 23 GPIO_ACTIVE_HIGH>;
+   write-enable;
+};
-- 
2.14.1

[PATCH v9 1/2] serio: PS/2 gpio bit banging driver for serio bus

[Danilo Krummrich]

This driver provides PS/2 serio bus support by implementing bit banging
with the GPIO API. The GPIO pins, data and clock, can be configured with
a node in the device tree or by generic device properties (GDP).

Writing to a device is supported as well, though it is possible timings
can not be halt as they are tough and difficult to reach with bit banging.
Therefore it can be configured (also in DT and GDP) whether the serio
write function should be available for clients.

This driver is for development purposes and not recommended for productive
use. However, this driver can be useful e.g. when no USB port is available
or using old peripherals is desired as PS/2 controller chips getting rare.

This driver was tested on bcm2825 and on Kirin 960 and it worked well
together with the atkbd and psmouse driver.

Signed-off-by: Danilo Krummrich <danilokrummr...@dk-develop.de>
---
 Documentation/gpio/drivers-on-gpio.txt |   5 +
 drivers/input/serio/Kconfig|  11 +
 drivers/input/serio/Makefile   |   1 +
 drivers/input/serio/ps2-gpio.c | 453 +
 4 files changed, 470 insertions(+)
 create mode 100644 drivers/input/serio/ps2-gpio.c

diff --git a/Documentation/gpio/drivers-on-gpio.txt 
b/Documentation/gpio/drivers-on-gpio.txt
index 306513251713..9a78d385b92e 100644
--- a/Documentation/gpio/drivers-on-gpio.txt
+++ b/Documentation/gpio/drivers-on-gpio.txt
@@ -84,6 +84,11 @@ hardware descriptions such as device tree or ACPI:
   NAND flash MTD subsystem and provides chip access and partition parsing like
   any other NAND driving hardware.
 
+- ps2-gpio: drivers/input/serio/ps2-gpio.c is used to drive a PS/2 (IBM) serio
+  bus, data and clock line, by bit banging two GPIO lines. It will appear as
+  any other serio bus to the system and makes it possible to connect drivers
+  for e.g. keyboards and other PS/2 protocol based devices.
+
 Apart from this there are special GPIO drivers in subsystems like MMC/SD to
 read card detect and write protect GPIO lines, and in the TTY serial subsystem
 to emulate MCTRL (modem control) signals CTS/RTS by using two GPIO lines. The
diff --git a/drivers/input/serio/Kconfig b/drivers/input/serio/Kconfig
index c3d05b4d3118..21488c048fa3 100644
--- a/drivers/input/serio/Kconfig
+++ b/drivers/input/serio/Kconfig
@@ -292,6 +292,17 @@ config SERIO_SUN4I_PS2
  To compile this driver as a module, choose M here: the
  module will be called sun4i-ps2.
 
+config SERIO_GPIO_PS2
+   tristate "GPIO PS/2 bit banging driver"
+   depends on GPIOLIB
+   help
+ Say Y here if you want PS/2 bit banging support via GPIO.
+
+ To compile this driver as a module, choose M here: the
+ module will be called ps2-gpio.
+
+ If you are unsure, say N.
+
 config USERIO
tristate "User space serio port driver support"
help
diff --git a/drivers/input/serio/Makefile b/drivers/input/serio/Makefile
index 2374ef9b33d7..767bd9b6e1ed 100644
--- a/drivers/input/serio/Makefile
+++ b/drivers/input/serio/Makefile
@@ -30,4 +30,5 @@ obj-$(CONFIG_SERIO_APBPS2)+= apbps2.o
 obj-$(CONFIG_SERIO_OLPC_APSP)  += olpc_apsp.o
 obj-$(CONFIG_HYPERV_KEYBOARD)  += hyperv-keyboard.o
 obj-$(CONFIG_SERIO_SUN4I_PS2)  += sun4i-ps2.o
+obj-$(CONFIG_SERIO_GPIO_PS2)   += ps2-gpio.o
 obj-$(CONFIG_USERIO)   += userio.o
diff --git a/drivers/input/serio/ps2-gpio.c b/drivers/input/serio/ps2-gpio.c
new file mode 100644
index ..b50e3817f3c4
--- /dev/null
+++ b/drivers/input/serio/ps2-gpio.c
@@ -0,0 +1,453 @@
+/*
+ * GPIO based serio bus driver for bit banging the PS/2 protocol
+ *
+ * Author: Danilo Krummrich <danilokrummr...@dk-develop.de>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define DRIVER_NAME"ps2-gpio"
+
+#define PS2_MODE_RX0
+#define PS2_MODE_TX1
+
+#define PS2_START_BIT  0
+#define PS2_DATA_BIT0  1
+#define PS2_DATA_BIT1  2
+#define PS2_DATA_BIT2  3
+#define PS2_DATA_BIT3  4
+#define PS2_DATA_BIT4  5
+#define PS2_DATA_BIT5  6
+#define PS2_DATA_BIT6  7
+#define PS2_DATA_BIT7  8
+#define PS2_PARITY_BIT 9
+#define PS2_STOP_BIT   10
+#define PS2_TX_TIMEOUT 11
+#define PS2_ACK_BIT12
+
+#define PS2_DEV_RET_ACK0xfa
+#define PS2_DEV_RET_NACK   0xfe
+
+#define PS2_CMD_RESEND 0xfe
+
+struct ps2_gpio_data {
+   struct device *dev;
+   struct serio *serio;
+   unsigned char mode;
+   struct gpio_desc *gpio_clk;
+   struct gpio_desc *gpio_data;
+   bool write_enab

[PATCH v9 1/2] serio: PS/2 gpio bit banging driver for serio bus

[Danilo Krummrich]

This driver provides PS/2 serio bus support by implementing bit banging
with the GPIO API. The GPIO pins, data and clock, can be configured with
a node in the device tree or by generic device properties (GDP).

Writing to a device is supported as well, though it is possible timings
can not be halt as they are tough and difficult to reach with bit banging.
Therefore it can be configured (also in DT and GDP) whether the serio
write function should be available for clients.

This driver is for development purposes and not recommended for productive
use. However, this driver can be useful e.g. when no USB port is available
or using old peripherals is desired as PS/2 controller chips getting rare.

This driver was tested on bcm2825 and on Kirin 960 and it worked well
together with the atkbd and psmouse driver.

Signed-off-by: Danilo Krummrich 
---
 Documentation/gpio/drivers-on-gpio.txt |   5 +
 drivers/input/serio/Kconfig|  11 +
 drivers/input/serio/Makefile   |   1 +
 drivers/input/serio/ps2-gpio.c | 453 +
 4 files changed, 470 insertions(+)
 create mode 100644 drivers/input/serio/ps2-gpio.c

diff --git a/Documentation/gpio/drivers-on-gpio.txt 
b/Documentation/gpio/drivers-on-gpio.txt
index 306513251713..9a78d385b92e 100644
--- a/Documentation/gpio/drivers-on-gpio.txt
+++ b/Documentation/gpio/drivers-on-gpio.txt
@@ -84,6 +84,11 @@ hardware descriptions such as device tree or ACPI:
   NAND flash MTD subsystem and provides chip access and partition parsing like
   any other NAND driving hardware.
 
+- ps2-gpio: drivers/input/serio/ps2-gpio.c is used to drive a PS/2 (IBM) serio
+  bus, data and clock line, by bit banging two GPIO lines. It will appear as
+  any other serio bus to the system and makes it possible to connect drivers
+  for e.g. keyboards and other PS/2 protocol based devices.
+
 Apart from this there are special GPIO drivers in subsystems like MMC/SD to
 read card detect and write protect GPIO lines, and in the TTY serial subsystem
 to emulate MCTRL (modem control) signals CTS/RTS by using two GPIO lines. The
diff --git a/drivers/input/serio/Kconfig b/drivers/input/serio/Kconfig
index c3d05b4d3118..21488c048fa3 100644
--- a/drivers/input/serio/Kconfig
+++ b/drivers/input/serio/Kconfig
@@ -292,6 +292,17 @@ config SERIO_SUN4I_PS2
  To compile this driver as a module, choose M here: the
  module will be called sun4i-ps2.
 
+config SERIO_GPIO_PS2
+   tristate "GPIO PS/2 bit banging driver"
+   depends on GPIOLIB
+   help
+ Say Y here if you want PS/2 bit banging support via GPIO.
+
+ To compile this driver as a module, choose M here: the
+ module will be called ps2-gpio.
+
+ If you are unsure, say N.
+
 config USERIO
tristate "User space serio port driver support"
help
diff --git a/drivers/input/serio/Makefile b/drivers/input/serio/Makefile
index 2374ef9b33d7..767bd9b6e1ed 100644
--- a/drivers/input/serio/Makefile
+++ b/drivers/input/serio/Makefile
@@ -30,4 +30,5 @@ obj-$(CONFIG_SERIO_APBPS2)+= apbps2.o
 obj-$(CONFIG_SERIO_OLPC_APSP)  += olpc_apsp.o
 obj-$(CONFIG_HYPERV_KEYBOARD)  += hyperv-keyboard.o
 obj-$(CONFIG_SERIO_SUN4I_PS2)  += sun4i-ps2.o
+obj-$(CONFIG_SERIO_GPIO_PS2)   += ps2-gpio.o
 obj-$(CONFIG_USERIO)   += userio.o
diff --git a/drivers/input/serio/ps2-gpio.c b/drivers/input/serio/ps2-gpio.c
new file mode 100644
index ..b50e3817f3c4
--- /dev/null
+++ b/drivers/input/serio/ps2-gpio.c
@@ -0,0 +1,453 @@
+/*
+ * GPIO based serio bus driver for bit banging the PS/2 protocol
+ *
+ * Author: Danilo Krummrich 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define DRIVER_NAME"ps2-gpio"
+
+#define PS2_MODE_RX0
+#define PS2_MODE_TX1
+
+#define PS2_START_BIT  0
+#define PS2_DATA_BIT0  1
+#define PS2_DATA_BIT1  2
+#define PS2_DATA_BIT2  3
+#define PS2_DATA_BIT3  4
+#define PS2_DATA_BIT4  5
+#define PS2_DATA_BIT5  6
+#define PS2_DATA_BIT6  7
+#define PS2_DATA_BIT7  8
+#define PS2_PARITY_BIT 9
+#define PS2_STOP_BIT   10
+#define PS2_TX_TIMEOUT 11
+#define PS2_ACK_BIT12
+
+#define PS2_DEV_RET_ACK0xfa
+#define PS2_DEV_RET_NACK   0xfe
+
+#define PS2_CMD_RESEND 0xfe
+
+struct ps2_gpio_data {
+   struct device *dev;
+   struct serio *serio;
+   unsigned char mode;
+   struct gpio_desc *gpio_clk;
+   struct gpio_desc *gpio_data;
+   bool write_enable;
+   int irq;
+   unsigned char rx_cnt;
+   unsigned char 

[PATCH v9 0/2] serio: PS/2 gpio bit banging driver for serio bus

[Danilo Krummrich]

v2: Removed one verbose print statement, changed another one to dev_dbg.
v3: - fixed compiler warning on blackfin
- depends on GPIOLIB
- clarify documentation
v4: - fixed concurrent calls to ps2_gpio_write (serio->write)
- use gpiod API·
- use generic device properties
- request irq separately, do not use gpiod_to_irq
- abort when gpio is connected via slow bus·
- Fixed a bug where PS2_CMD_RESEND is always send after tx failed once.
  The makes the write functionallity work better, tough timing is still
  critical.
- disable irq initially until ps2_gpip_open (serio->open) is called
v5: Checked again why timings are that hard to reach while in tx mode and·
discovered that there is an extra clock pulse between stop bit sent from
host and acknowledgement from device. By just skipping this clock pulse
tx works fine now, though it still happens sometimes that the timing can·
not be reached of course.
v6: - fixed typos
- use of_match_ptr
v7: remove unnecessary barriers
v8: - split patch to have a separate one for the dt binding
- remove ps2-gpio prefix in binding
v9: In "dt-bindings: new binding for ps/2 gpio devices" required property list
was not matching the example node.

Danilo Krummrich (2):
  serio: PS/2 gpio bit banging driver for serio bus
  dt-bindings: new binding for ps/2 gpio devices

 .../devicetree/bindings/serio/ps2-gpio.txt |  23 ++
 Documentation/gpio/drivers-on-gpio.txt |   5 +
 drivers/input/serio/Kconfig|  11 +
 drivers/input/serio/Makefile   |   1 +
 drivers/input/serio/ps2-gpio.c | 453 +
 5 files changed, 493 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/serio/ps2-gpio.txt
 create mode 100644 drivers/input/serio/ps2-gpio.c

-- 
2.14.1

[PATCH v9 0/2] serio: PS/2 gpio bit banging driver for serio bus

[Danilo Krummrich]

v2: Removed one verbose print statement, changed another one to dev_dbg.
v3: - fixed compiler warning on blackfin
- depends on GPIOLIB
- clarify documentation
v4: - fixed concurrent calls to ps2_gpio_write (serio->write)
- use gpiod API·
- use generic device properties
- request irq separately, do not use gpiod_to_irq
- abort when gpio is connected via slow bus·
- Fixed a bug where PS2_CMD_RESEND is always send after tx failed once.
  The makes the write functionallity work better, tough timing is still
  critical.
- disable irq initially until ps2_gpip_open (serio->open) is called
v5: Checked again why timings are that hard to reach while in tx mode and·
discovered that there is an extra clock pulse between stop bit sent from
host and acknowledgement from device. By just skipping this clock pulse
tx works fine now, though it still happens sometimes that the timing can·
not be reached of course.
v6: - fixed typos
- use of_match_ptr
v7: remove unnecessary barriers
v8: - split patch to have a separate one for the dt binding
- remove ps2-gpio prefix in binding
v9: In "dt-bindings: new binding for ps/2 gpio devices" required property list
was not matching the example node.

Danilo Krummrich (2):
  serio: PS/2 gpio bit banging driver for serio bus
  dt-bindings: new binding for ps/2 gpio devices

 .../devicetree/bindings/serio/ps2-gpio.txt |  23 ++
 Documentation/gpio/drivers-on-gpio.txt |   5 +
 drivers/input/serio/Kconfig|  11 +
 drivers/input/serio/Makefile   |   1 +
 drivers/input/serio/ps2-gpio.c | 453 +
 5 files changed, 493 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/serio/ps2-gpio.txt
 create mode 100644 drivers/input/serio/ps2-gpio.c

-- 
2.14.1

[PATCH v8 2/2] dt-bindings: new binding for ps/2 gpio devices

[Danilo Krummrich]

The PS/2 gpio device binding defines the gpio pins (data and clock)
as well as the interrupt which should be used to drive the ps/2 bus.
It is expected to get an interrupt on the falling edge of the clock
line.

Also it can be configured whether the host should support writing to
the device.

Signed-off-by: Danilo Krummrich <danilokrummr...@dk-develop.de>
---
 .../devicetree/bindings/serio/ps2-gpio.txt | 22 ++
 1 file changed, 22 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/serio/ps2-gpio.txt

diff --git a/Documentation/devicetree/bindings/serio/ps2-gpio.txt 
b/Documentation/devicetree/bindings/serio/ps2-gpio.txt
new file mode 100644
index ..83e05ea05883
--- /dev/null
+++ b/Documentation/devicetree/bindings/serio/ps2-gpio.txt
@@ -0,0 +1,22 @@
+Device-Tree binding for ps/2 gpio device
+
+Required properties:
+   - compatible = "ps2-gpio"
+   - gpios: data and clock gpio
+   - interrupts: Should trigger on the falling edge of the clock line.
+
+Optional properties:
+   - write-enable: Indicates whether write function is provided
+   to serio device. Possibly providing the write fn will not work, because
+   of the tough timing requirements.
+
+Example nodes:
+
+ps2@0 {
+   compatible = "ps2-gpio";
+   interrupt-parent = <>;
+   interrupts = <23 IRQ_TYPE_EDGE_FALLING>;
+   data-gpios = < 24 GPIO_ACTIVE_HIGH>;
+   clk-gpios = < 23 GPIO_ACTIVE_HIGH>;
+   write-enable;
+};
-- 
2.14.1

[PATCH v8 2/2] dt-bindings: new binding for ps/2 gpio devices

[Danilo Krummrich]

The PS/2 gpio device binding defines the gpio pins (data and clock)
as well as the interrupt which should be used to drive the ps/2 bus.
It is expected to get an interrupt on the falling edge of the clock
line.

Also it can be configured whether the host should support writing to
the device.

Signed-off-by: Danilo Krummrich 
---
 .../devicetree/bindings/serio/ps2-gpio.txt | 22 ++
 1 file changed, 22 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/serio/ps2-gpio.txt

diff --git a/Documentation/devicetree/bindings/serio/ps2-gpio.txt 
b/Documentation/devicetree/bindings/serio/ps2-gpio.txt
new file mode 100644
index ..83e05ea05883
--- /dev/null
+++ b/Documentation/devicetree/bindings/serio/ps2-gpio.txt
@@ -0,0 +1,22 @@
+Device-Tree binding for ps/2 gpio device
+
+Required properties:
+   - compatible = "ps2-gpio"
+   - gpios: data and clock gpio
+   - interrupts: Should trigger on the falling edge of the clock line.
+
+Optional properties:
+   - write-enable: Indicates whether write function is provided
+   to serio device. Possibly providing the write fn will not work, because
+   of the tough timing requirements.
+
+Example nodes:
+
+ps2@0 {
+   compatible = "ps2-gpio";
+   interrupt-parent = <>;
+   interrupts = <23 IRQ_TYPE_EDGE_FALLING>;
+   data-gpios = < 24 GPIO_ACTIVE_HIGH>;
+   clk-gpios = < 23 GPIO_ACTIVE_HIGH>;
+   write-enable;
+};
-- 
2.14.1

[PATCH v8 1/2] serio: PS/2 gpio bit banging driver for serio bus

[Danilo Krummrich]

This driver provides PS/2 serio bus support by implementing bit banging
with the GPIO API. The GPIO pins, data and clock, can be configured with
a node in the device tree or by generic device properties (GDP).

Writing to a device is supported as well, though it is possible timings
can not be halt as they are tough and difficult to reach with bit banging.
Therefore it can be configured (also in DT and GDP) whether the serio
write function should be available for clients.

This driver is for development purposes and not recommended for productive
use. However, this driver can be useful e.g. when no USB port is available
or using old peripherals is desired as PS/2 controller chips getting rare.

This driver was tested on bcm2825 and on Kirin 960 and it worked well
together with the atkbd and psmouse driver.

Signed-off-by: Danilo Krummrich <danilokrummr...@dk-develop.de>
---
 Documentation/gpio/drivers-on-gpio.txt |   5 +
 drivers/input/serio/Kconfig|  11 +
 drivers/input/serio/Makefile   |   1 +
 drivers/input/serio/ps2-gpio.c | 453 +
 4 files changed, 470 insertions(+)
 create mode 100644 drivers/input/serio/ps2-gpio.c

diff --git a/Documentation/gpio/drivers-on-gpio.txt 
b/Documentation/gpio/drivers-on-gpio.txt
index 306513251713..9a78d385b92e 100644
--- a/Documentation/gpio/drivers-on-gpio.txt
+++ b/Documentation/gpio/drivers-on-gpio.txt
@@ -84,6 +84,11 @@ hardware descriptions such as device tree or ACPI:
   NAND flash MTD subsystem and provides chip access and partition parsing like
   any other NAND driving hardware.
 
+- ps2-gpio: drivers/input/serio/ps2-gpio.c is used to drive a PS/2 (IBM) serio
+  bus, data and clock line, by bit banging two GPIO lines. It will appear as
+  any other serio bus to the system and makes it possible to connect drivers
+  for e.g. keyboards and other PS/2 protocol based devices.
+
 Apart from this there are special GPIO drivers in subsystems like MMC/SD to
 read card detect and write protect GPIO lines, and in the TTY serial subsystem
 to emulate MCTRL (modem control) signals CTS/RTS by using two GPIO lines. The
diff --git a/drivers/input/serio/Kconfig b/drivers/input/serio/Kconfig
index c3d05b4d3118..21488c048fa3 100644
--- a/drivers/input/serio/Kconfig
+++ b/drivers/input/serio/Kconfig
@@ -292,6 +292,17 @@ config SERIO_SUN4I_PS2
  To compile this driver as a module, choose M here: the
  module will be called sun4i-ps2.
 
+config SERIO_GPIO_PS2
+   tristate "GPIO PS/2 bit banging driver"
+   depends on GPIOLIB
+   help
+ Say Y here if you want PS/2 bit banging support via GPIO.
+
+ To compile this driver as a module, choose M here: the
+ module will be called ps2-gpio.
+
+ If you are unsure, say N.
+
 config USERIO
tristate "User space serio port driver support"
help
diff --git a/drivers/input/serio/Makefile b/drivers/input/serio/Makefile
index 2374ef9b33d7..767bd9b6e1ed 100644
--- a/drivers/input/serio/Makefile
+++ b/drivers/input/serio/Makefile
@@ -30,4 +30,5 @@ obj-$(CONFIG_SERIO_APBPS2)+= apbps2.o
 obj-$(CONFIG_SERIO_OLPC_APSP)  += olpc_apsp.o
 obj-$(CONFIG_HYPERV_KEYBOARD)  += hyperv-keyboard.o
 obj-$(CONFIG_SERIO_SUN4I_PS2)  += sun4i-ps2.o
+obj-$(CONFIG_SERIO_GPIO_PS2)   += ps2-gpio.o
 obj-$(CONFIG_USERIO)   += userio.o
diff --git a/drivers/input/serio/ps2-gpio.c b/drivers/input/serio/ps2-gpio.c
new file mode 100644
index ..b50e3817f3c4
--- /dev/null
+++ b/drivers/input/serio/ps2-gpio.c
@@ -0,0 +1,453 @@
+/*
+ * GPIO based serio bus driver for bit banging the PS/2 protocol
+ *
+ * Author: Danilo Krummrich <danilokrummr...@dk-develop.de>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define DRIVER_NAME"ps2-gpio"
+
+#define PS2_MODE_RX0
+#define PS2_MODE_TX1
+
+#define PS2_START_BIT  0
+#define PS2_DATA_BIT0  1
+#define PS2_DATA_BIT1  2
+#define PS2_DATA_BIT2  3
+#define PS2_DATA_BIT3  4
+#define PS2_DATA_BIT4  5
+#define PS2_DATA_BIT5  6
+#define PS2_DATA_BIT6  7
+#define PS2_DATA_BIT7  8
+#define PS2_PARITY_BIT 9
+#define PS2_STOP_BIT   10
+#define PS2_TX_TIMEOUT 11
+#define PS2_ACK_BIT12
+
+#define PS2_DEV_RET_ACK0xfa
+#define PS2_DEV_RET_NACK   0xfe
+
+#define PS2_CMD_RESEND 0xfe
+
+struct ps2_gpio_data {
+   struct device *dev;
+   struct serio *serio;
+   unsigned char mode;
+   struct gpio_desc *gpio_clk;
+   struct gpio_desc *gpio_data;
+   bool write_enab

[PATCH v8 1/2] serio: PS/2 gpio bit banging driver for serio bus

[Danilo Krummrich]

This driver provides PS/2 serio bus support by implementing bit banging
with the GPIO API. The GPIO pins, data and clock, can be configured with
a node in the device tree or by generic device properties (GDP).

Writing to a device is supported as well, though it is possible timings
can not be halt as they are tough and difficult to reach with bit banging.
Therefore it can be configured (also in DT and GDP) whether the serio
write function should be available for clients.

This driver is for development purposes and not recommended for productive
use. However, this driver can be useful e.g. when no USB port is available
or using old peripherals is desired as PS/2 controller chips getting rare.

This driver was tested on bcm2825 and on Kirin 960 and it worked well
together with the atkbd and psmouse driver.

Signed-off-by: Danilo Krummrich 
---
 Documentation/gpio/drivers-on-gpio.txt |   5 +
 drivers/input/serio/Kconfig|  11 +
 drivers/input/serio/Makefile   |   1 +
 drivers/input/serio/ps2-gpio.c | 453 +
 4 files changed, 470 insertions(+)
 create mode 100644 drivers/input/serio/ps2-gpio.c

diff --git a/Documentation/gpio/drivers-on-gpio.txt 
b/Documentation/gpio/drivers-on-gpio.txt
index 306513251713..9a78d385b92e 100644
--- a/Documentation/gpio/drivers-on-gpio.txt
+++ b/Documentation/gpio/drivers-on-gpio.txt
@@ -84,6 +84,11 @@ hardware descriptions such as device tree or ACPI:
   NAND flash MTD subsystem and provides chip access and partition parsing like
   any other NAND driving hardware.
 
+- ps2-gpio: drivers/input/serio/ps2-gpio.c is used to drive a PS/2 (IBM) serio
+  bus, data and clock line, by bit banging two GPIO lines. It will appear as
+  any other serio bus to the system and makes it possible to connect drivers
+  for e.g. keyboards and other PS/2 protocol based devices.
+
 Apart from this there are special GPIO drivers in subsystems like MMC/SD to
 read card detect and write protect GPIO lines, and in the TTY serial subsystem
 to emulate MCTRL (modem control) signals CTS/RTS by using two GPIO lines. The
diff --git a/drivers/input/serio/Kconfig b/drivers/input/serio/Kconfig
index c3d05b4d3118..21488c048fa3 100644
--- a/drivers/input/serio/Kconfig
+++ b/drivers/input/serio/Kconfig
@@ -292,6 +292,17 @@ config SERIO_SUN4I_PS2
  To compile this driver as a module, choose M here: the
  module will be called sun4i-ps2.
 
+config SERIO_GPIO_PS2
+   tristate "GPIO PS/2 bit banging driver"
+   depends on GPIOLIB
+   help
+ Say Y here if you want PS/2 bit banging support via GPIO.
+
+ To compile this driver as a module, choose M here: the
+ module will be called ps2-gpio.
+
+ If you are unsure, say N.
+
 config USERIO
tristate "User space serio port driver support"
help
diff --git a/drivers/input/serio/Makefile b/drivers/input/serio/Makefile
index 2374ef9b33d7..767bd9b6e1ed 100644
--- a/drivers/input/serio/Makefile
+++ b/drivers/input/serio/Makefile
@@ -30,4 +30,5 @@ obj-$(CONFIG_SERIO_APBPS2)+= apbps2.o
 obj-$(CONFIG_SERIO_OLPC_APSP)  += olpc_apsp.o
 obj-$(CONFIG_HYPERV_KEYBOARD)  += hyperv-keyboard.o
 obj-$(CONFIG_SERIO_SUN4I_PS2)  += sun4i-ps2.o
+obj-$(CONFIG_SERIO_GPIO_PS2)   += ps2-gpio.o
 obj-$(CONFIG_USERIO)   += userio.o
diff --git a/drivers/input/serio/ps2-gpio.c b/drivers/input/serio/ps2-gpio.c
new file mode 100644
index ..b50e3817f3c4
--- /dev/null
+++ b/drivers/input/serio/ps2-gpio.c
@@ -0,0 +1,453 @@
+/*
+ * GPIO based serio bus driver for bit banging the PS/2 protocol
+ *
+ * Author: Danilo Krummrich 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define DRIVER_NAME"ps2-gpio"
+
+#define PS2_MODE_RX0
+#define PS2_MODE_TX1
+
+#define PS2_START_BIT  0
+#define PS2_DATA_BIT0  1
+#define PS2_DATA_BIT1  2
+#define PS2_DATA_BIT2  3
+#define PS2_DATA_BIT3  4
+#define PS2_DATA_BIT4  5
+#define PS2_DATA_BIT5  6
+#define PS2_DATA_BIT6  7
+#define PS2_DATA_BIT7  8
+#define PS2_PARITY_BIT 9
+#define PS2_STOP_BIT   10
+#define PS2_TX_TIMEOUT 11
+#define PS2_ACK_BIT12
+
+#define PS2_DEV_RET_ACK0xfa
+#define PS2_DEV_RET_NACK   0xfe
+
+#define PS2_CMD_RESEND 0xfe
+
+struct ps2_gpio_data {
+   struct device *dev;
+   struct serio *serio;
+   unsigned char mode;
+   struct gpio_desc *gpio_clk;
+   struct gpio_desc *gpio_data;
+   bool write_enable;
+   int irq;
+   unsigned char rx_cnt;
+   unsigned char 

[PATCH v8 0/2] serio: PS/2 gpio bit banging driver for serio bus

[Danilo Krummrich]

v2: Removed one verbose print statement, changed another one to dev_dbg.
v3: - fixed compiler warning on blackfin
- depends on GPIOLIB
- clarify documentation
v4: - fixed concurrent calls to ps2_gpio_write (serio->write)
- use gpiod API·
- use generic device properties
- request irq separately, do not use gpiod_to_irq
- abort when gpio is connected via slow bus·
- Fixed a bug where PS2_CMD_RESEND is always send after tx failed once.
  The makes the write functionallity work better, tough timing is still
  critical.
- disable irq initially until ps2_gpip_open (serio->open) is called
v5: Checked again why timings are that hard to reach while in tx mode and·
discovered that there is an extra clock pulse between stop bit sent from
host and acknowledgement from device. By just skipping this clock pulse
tx works fine now, though it still happens sometimes that the timing can·
not be reached of course.
v6: - fixed typos
- use of_match_ptr
v7: remove unnecessary barriers
v8: - split patch to have a separate one for the dt binding
- remove ps2-gpio prefix in binding

Danilo Krummrich (2):
  serio: PS/2 gpio bit banging driver for serio bus
  dt-bindings: new binding for ps/2 gpio devices

 .../devicetree/bindings/serio/ps2-gpio.txt |  22 +
 Documentation/gpio/drivers-on-gpio.txt |   5 +
 drivers/input/serio/Kconfig|  11 +
 drivers/input/serio/Makefile   |   1 +
 drivers/input/serio/ps2-gpio.c | 453 +
 5 files changed, 492 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/serio/ps2-gpio.txt
 create mode 100644 drivers/input/serio/ps2-gpio.c

--
2.14.1

[PATCH v8 0/2] serio: PS/2 gpio bit banging driver for serio bus

[Danilo Krummrich]

v2: Removed one verbose print statement, changed another one to dev_dbg.
v3: - fixed compiler warning on blackfin
- depends on GPIOLIB
- clarify documentation
v4: - fixed concurrent calls to ps2_gpio_write (serio->write)
- use gpiod API·
- use generic device properties
- request irq separately, do not use gpiod_to_irq
- abort when gpio is connected via slow bus·
- Fixed a bug where PS2_CMD_RESEND is always send after tx failed once.
  The makes the write functionallity work better, tough timing is still
  critical.
- disable irq initially until ps2_gpip_open (serio->open) is called
v5: Checked again why timings are that hard to reach while in tx mode and·
discovered that there is an extra clock pulse between stop bit sent from
host and acknowledgement from device. By just skipping this clock pulse
tx works fine now, though it still happens sometimes that the timing can·
not be reached of course.
v6: - fixed typos
- use of_match_ptr
v7: remove unnecessary barriers
v8: - split patch to have a separate one for the dt binding
- remove ps2-gpio prefix in binding

Danilo Krummrich (2):
  serio: PS/2 gpio bit banging driver for serio bus
  dt-bindings: new binding for ps/2 gpio devices

 .../devicetree/bindings/serio/ps2-gpio.txt |  22 +
 Documentation/gpio/drivers-on-gpio.txt |   5 +
 drivers/input/serio/Kconfig|  11 +
 drivers/input/serio/Makefile   |   1 +
 drivers/input/serio/ps2-gpio.c | 453 +
 5 files changed, 492 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/serio/ps2-gpio.txt
 create mode 100644 drivers/input/serio/ps2-gpio.c

--
2.14.1

Re: [PATCH v7] serio: PS/2 gpio bit banging driver for serio bus

[Danilo Krummrich]

Hi Bob,

thanks for reviewing.

On 2017-08-17 17:43, Rob Herring wrote:

On Fri, Aug 11, 2017 at 03:17:36PM +0200, Danilo Krummrich wrote:
This driver provides PS/2 serio bus support by implementing bit 
banging
with the GPIO API. The GPIO pins, data and clock, can be configured 
with

a node in the device tree or by generic device properties (GDP).

Writing to a device is supported as well, though it is possible 
timings
can not be halt as they are tough and difficult to reach with bit 
banging.

Therefore it can be configured (also in DT and GDP) whether the serio
write function should be available for clients.

This driver is for development purposes and not recommended for 
productive
use. However, this driver can be useful e.g. when no USB port is 
available
or using old peripherals is desired as PS/2 controller chips getting 
rare.


This driver was tested on RPI1 and on Hikey960 and it worked well 
together

with the atkbd and psmouse driver.

Signed-off-by: Danilo Krummrich <danilokrummr...@dk-develop.de>
---
v2: Removed one verbose print statement, changed another one to 
dev_dbg.

v3: - fixed compiler warning on blackfin
- depends on GPIOLIB
- clarify documentation
v4: - fixed concurrent calls to ps2_gpio_write (serio->write)
- use gpiod API
- use generic device properties
- request irq separately, do not use gpiod_to_irq
- abort when gpio is connected via slow bus
- Fixed a bug where PS2_CMD_RESEND is always send after tx failed 
once.
  The makes the write functionallity work better, tough timing is 
still

  critical.
- disable irq initially until ps2_gpip_open (serio->open) is 
called
v5: Checked again why timings are that hard to reach while in tx mode 
and
discovered that there is an extra clock pulse between stop bit 
sent from
host and acknowledgement from device. By just skipping this clock 
pulse
tx works fine now, though it still happens sometimes that the 
timing can

not be reached of course.
v6: - fixed typos
- use of_match_ptr
v7: remove unnecessary barriers

Sorry for resending, forgot der version tag in the subject.
---
 .../devicetree/bindings/serio/ps2-gpio.txt |  22 +


It's preferred to split bindings to separate patch.

Together with Documentation/gpio/drivers-on-gpio.txt or would you prefer 
to have

a separate patch for this as well?

 Documentation/gpio/drivers-on-gpio.txt |   5 +
 drivers/input/serio/Kconfig|  11 +
 drivers/input/serio/Makefile   |   1 +
 drivers/input/serio/ps2-gpio.c | 453 
+

 5 files changed, 492 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/serio/ps2-gpio.txt

 create mode 100644 drivers/input/serio/ps2-gpio.c

diff --git a/Documentation/devicetree/bindings/serio/ps2-gpio.txt 
b/Documentation/devicetree/bindings/serio/ps2-gpio.txt

new file mode 100644
index ..099dd6d46cb3
--- /dev/null
+++ b/Documentation/devicetree/bindings/serio/ps2-gpio.txt
@@ -0,0 +1,22 @@
+Device-Tree bindings for ps/2 gpio driver


Bindings don't describe drivers.


Will fix.

+
+Required properties:
+   - compatible = "ps2-gpio"
+   - gpios: data and clock gpio
+   - interrupts: Should trigger on the falling edge of the clock line.
+
+Optional properties:
+	- ps2-gpio,write-enable: Indicates whether write function is 
provided


ps2-gpio is not a vendor prefix, so drop it.


I will do.
+	to serio device. Possibly providing the write fn will not work, 
because

+   of the tough timing requirements.
+
+Example nodes:
+
+ps2@0 {
+   compatible = "ps2-gpio";
+   interrupt-parent = <>;
+   interrupts = <23 IRQ_TYPE_EDGE_FALLING>;
+   data-gpios = < 24 GPIO_ACTIVE_HIGH>;
+   clk-gpios = < 23 GPIO_ACTIVE_HIGH>;
+   ps2-gpio,write-enable;
+};


Thanks,
Danilo

Re: [PATCH v7] serio: PS/2 gpio bit banging driver for serio bus

[Danilo Krummrich]

Hi Bob,

thanks for reviewing.

On 2017-08-17 17:43, Rob Herring wrote:

On Fri, Aug 11, 2017 at 03:17:36PM +0200, Danilo Krummrich wrote:
This driver provides PS/2 serio bus support by implementing bit 
banging
with the GPIO API. The GPIO pins, data and clock, can be configured 
with

a node in the device tree or by generic device properties (GDP).

Writing to a device is supported as well, though it is possible 
timings
can not be halt as they are tough and difficult to reach with bit 
banging.

Therefore it can be configured (also in DT and GDP) whether the serio
write function should be available for clients.

This driver is for development purposes and not recommended for 
productive
use. However, this driver can be useful e.g. when no USB port is 
available
or using old peripherals is desired as PS/2 controller chips getting 
rare.


This driver was tested on RPI1 and on Hikey960 and it worked well 
together

with the atkbd and psmouse driver.

Signed-off-by: Danilo Krummrich 
---
v2: Removed one verbose print statement, changed another one to 
dev_dbg.

v3: - fixed compiler warning on blackfin
- depends on GPIOLIB
- clarify documentation
v4: - fixed concurrent calls to ps2_gpio_write (serio->write)
- use gpiod API
- use generic device properties
- request irq separately, do not use gpiod_to_irq
- abort when gpio is connected via slow bus
- Fixed a bug where PS2_CMD_RESEND is always send after tx failed 
once.
  The makes the write functionallity work better, tough timing is 
still

  critical.
- disable irq initially until ps2_gpip_open (serio->open) is 
called
v5: Checked again why timings are that hard to reach while in tx mode 
and
discovered that there is an extra clock pulse between stop bit 
sent from
host and acknowledgement from device. By just skipping this clock 
pulse
tx works fine now, though it still happens sometimes that the 
timing can

not be reached of course.
v6: - fixed typos
- use of_match_ptr
v7: remove unnecessary barriers

Sorry for resending, forgot der version tag in the subject.
---
 .../devicetree/bindings/serio/ps2-gpio.txt |  22 +


It's preferred to split bindings to separate patch.

Together with Documentation/gpio/drivers-on-gpio.txt or would you prefer 
to have

a separate patch for this as well?

 Documentation/gpio/drivers-on-gpio.txt |   5 +
 drivers/input/serio/Kconfig|  11 +
 drivers/input/serio/Makefile   |   1 +
 drivers/input/serio/ps2-gpio.c | 453 
+

 5 files changed, 492 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/serio/ps2-gpio.txt

 create mode 100644 drivers/input/serio/ps2-gpio.c

diff --git a/Documentation/devicetree/bindings/serio/ps2-gpio.txt 
b/Documentation/devicetree/bindings/serio/ps2-gpio.txt

new file mode 100644
index ..099dd6d46cb3
--- /dev/null
+++ b/Documentation/devicetree/bindings/serio/ps2-gpio.txt
@@ -0,0 +1,22 @@
+Device-Tree bindings for ps/2 gpio driver


Bindings don't describe drivers.


Will fix.

+
+Required properties:
+   - compatible = "ps2-gpio"
+   - gpios: data and clock gpio
+   - interrupts: Should trigger on the falling edge of the clock line.
+
+Optional properties:
+	- ps2-gpio,write-enable: Indicates whether write function is 
provided


ps2-gpio is not a vendor prefix, so drop it.


I will do.
+	to serio device. Possibly providing the write fn will not work, 
because

+   of the tough timing requirements.
+
+Example nodes:
+
+ps2@0 {
+   compatible = "ps2-gpio";
+   interrupt-parent = <>;
+   interrupts = <23 IRQ_TYPE_EDGE_FALLING>;
+   data-gpios = < 24 GPIO_ACTIVE_HIGH>;
+   clk-gpios = < 23 GPIO_ACTIVE_HIGH>;
+   ps2-gpio,write-enable;
+};


Thanks,
Danilo

Re: [PATCH] serio: PS2 gpio bit banging driver for the serio bus

[Danilo Krummrich]

On 2017-08-17 15:01, Russell King - ARM Linux wrote:

On Thu, Aug 17, 2017 at 12:51:33PM +0200, Danilo Krummrich wrote:
That having the correct execution order is not enough on some buses 
because
of buffering is really something to be aware of, thanks again for 
pointing

this out.


PCI guarantees the order of writes to a device, but there are 
situations

on SoCs where you can't rely on that - for instance, if the writes go
over different buses to different devices (eg, write to a peripheral
vs write to an interrupt controller.)

Even then, with interrupts delivered by message (eg, MSI) there's
issues.

So for the scenario I was concerned about I would expect the irqchip 
driver
guarantees the write actually hits the the hardware (if necessary read 
it
back) before the function (disable_irq_nosync()) returns, is that 
correct?

Though, having the need should be very unlikely.


Well, disable_irq_nosync() doesn't guarantee that the interrupt handler
isn't running - a CPU may have just received the interrupt and is just
entering the interrupt handler when disable_irq_nosync() returns.  The
hint is the "nosync" - there's no synchronisation.  If you need to
guarantee that the interrupt handler is not running, disable_irq() does
that.  By implication, however, disable_irq() can not be called from
within the same interrupt handler for the interrupt that is being
disabled.

Thanks again, I'm aware of that. As in my case the code could be called 
from

atomic context disable_irq() is not an option.

My main point is if it can be assumed that after disable_irq_nosync() 
returns
it is guaranteed, by convention, that the hardware was hit. But I really 
would

think so.

Re: [PATCH] serio: PS2 gpio bit banging driver for the serio bus

[Danilo Krummrich]

On 2017-08-17 15:01, Russell King - ARM Linux wrote:

On Thu, Aug 17, 2017 at 12:51:33PM +0200, Danilo Krummrich wrote:
That having the correct execution order is not enough on some buses 
because
of buffering is really something to be aware of, thanks again for 
pointing

this out.


PCI guarantees the order of writes to a device, but there are 
situations

on SoCs where you can't rely on that - for instance, if the writes go
over different buses to different devices (eg, write to a peripheral
vs write to an interrupt controller.)

Even then, with interrupts delivered by message (eg, MSI) there's
issues.

So for the scenario I was concerned about I would expect the irqchip 
driver
guarantees the write actually hits the the hardware (if necessary read 
it
back) before the function (disable_irq_nosync()) returns, is that 
correct?

Though, having the need should be very unlikely.


Well, disable_irq_nosync() doesn't guarantee that the interrupt handler
isn't running - a CPU may have just received the interrupt and is just
entering the interrupt handler when disable_irq_nosync() returns.  The
hint is the "nosync" - there's no synchronisation.  If you need to
guarantee that the interrupt handler is not running, disable_irq() does
that.  By implication, however, disable_irq() can not be called from
within the same interrupt handler for the interrupt that is being
disabled.

Thanks again, I'm aware of that. As in my case the code could be called 
from

atomic context disable_irq() is not an option.

My main point is if it can be assumed that after disable_irq_nosync() 
returns
it is guaranteed, by convention, that the hardware was hit. But I really 
would

think so.

Re: [PATCH] serio: PS2 gpio bit banging driver for the serio bus

[Danilo Krummrich]

On 2017-08-17 11:09, Russell King - ARM Linux wrote:

On Fri, Aug 11, 2017 at 11:16:20AM +0200, Linus Walleij wrote:
writel() should be guaranteeing that the values hit the hardware, 
wmb() is

spelled out "write memory barrier" I don't see what you're after here.


Incorrect.  writel() has a barrier which ensures that data written to
memory (eg, dma coherent memory) is visible to the hardware prior to
the write hitting the hardware.

There is no barrier to ensure that the write hits the hardware in a
timely manner - the write can be buffered by the buses, which will
delay it before it hits its destination.

PCI particularly buffers MMIO writes, and the requirement there has
always been that if you need the write to hit the hardware in a timely
fashion, you must perform a read-back to force the bus to deliver the
write (since a read is not allowed to overlap a write.)

The solution is never to use barrier() - barrier() is a _compiler_
barrier and does nothing for posted writes on hardware buses.


Thanks for clarification. I thought I just need a wmb() to make sure 
writel()
can not be reordered with another store operation. I wasn't aware that 
writel()

is defined to guarantee this on every arch.

That having the correct execution order is not enough on some buses 
because
of buffering is really something to be aware of, thanks again for 
pointing

this out.

So for the scenario I was concerned about I would expect the irqchip 
driver
guarantees the write actually hits the the hardware (if necessary read 
it
back) before the function (disable_irq_nosync()) returns, is that 
correct?

Though, having the need should be very unlikely.

Re: [PATCH] serio: PS2 gpio bit banging driver for the serio bus

[Danilo Krummrich]

On 2017-08-17 11:09, Russell King - ARM Linux wrote:

On Fri, Aug 11, 2017 at 11:16:20AM +0200, Linus Walleij wrote:
writel() should be guaranteeing that the values hit the hardware, 
wmb() is

spelled out "write memory barrier" I don't see what you're after here.


Incorrect.  writel() has a barrier which ensures that data written to
memory (eg, dma coherent memory) is visible to the hardware prior to
the write hitting the hardware.

There is no barrier to ensure that the write hits the hardware in a
timely manner - the write can be buffered by the buses, which will
delay it before it hits its destination.

PCI particularly buffers MMIO writes, and the requirement there has
always been that if you need the write to hit the hardware in a timely
fashion, you must perform a read-back to force the bus to deliver the
write (since a read is not allowed to overlap a write.)

The solution is never to use barrier() - barrier() is a _compiler_
barrier and does nothing for posted writes on hardware buses.


Thanks for clarification. I thought I just need a wmb() to make sure 
writel()
can not be reordered with another store operation. I wasn't aware that 
writel()

is defined to guarantee this on every arch.

That having the correct execution order is not enough on some buses 
because
of buffering is really something to be aware of, thanks again for 
pointing

this out.

So for the scenario I was concerned about I would expect the irqchip 
driver
guarantees the write actually hits the the hardware (if necessary read 
it
back) before the function (disable_irq_nosync()) returns, is that 
correct?

Though, having the need should be very unlikely.

Re: [PATCH] usb: gadget: udc: udc_stop before gadget unbind

[Danilo Krummrich]

On 2017-08-15 14:02, Felipe Balbi wrote:

Hi,

Danilo Krummrich <danilokrummr...@dk-develop.de> writes:

thanks for reviewing.


np :-)


On 2017-08-15 12:03, Felipe Balbi wrote:

Hi,

Danilo Krummrich <danilokrummr...@dk-develop.de> writes:
udc_stop needs to be called before gadget driver unbind. Otherwise 
it
might happen that udc drivers still call into the gadget driver 
(e.g.
to reset gadget after OTG event). If this happens it is likely to 
get
panics from gadget driver dereferencing NULL ptr, as gadget's 
drvdata

is set to NULL on unbind.


seems like the problem here is with the OTG layer, not UDC core.


I mentioned this just as example, it can happen whenever a UDC driver
calls
the gadget driver (e.g. by calling usb_gadget_udc_reset() in ISR) 
after

gadget
drivers unbind() was called already (e.g. by gadget configfs).
If this happens gadget drivers drvdata was already set to NULL by
unbind()
and reset() could result into a NULL ptr exception.
Therefore my assumption was that it needs to be prevented that the
gadget
driver is getting called after unbind.


We have a known problem in the design of the gadget API that causes 
this

races but we couldn't come up with a solution yet :-)

Inverting these two calls is not the correct way to go about this :-)


Now I see, thanks for explanation below.


Signed-off-by: Danilo Krummrich <danilokrummr...@dk-develop.de>
---
Actually there could still be a race:
(CPU1 code taken from dwc3 drivers dwc3_disconnect_gadget() as
exsample)

CPU0CPU1

usb_gadget_disconnect(udc->gadget);
udc->driver->disconnect(udc->gadget);
if (dwc->gadget_driver && 
dwc->gadget_driver->disconnect)
usb_gadget_udc_stop(udc);
udc->driver->unbind(udc->gadget);

dwc->gadget_driver->disconnect(>gadget);

UDC drivers typically set their gadget driver pointer to NULL in
udc_stop
and check for it before calling into the gadget driver. To fix the
issue
above every udc driver could apply a lock around this.

If you see the need for having this or another solutions I can 
provide

further patches. This patch could also just serve as a base for
discussion
if someone knows a smarter solution.

I saw this problem causing a panic on hikey960 board and provided a
quick
workaround for the same problem here:
https://android-review.googlesource.com/#/c/kernel/common/+/457476/
(panic log in the commit message of the linked patch)
---
 drivers/usb/gadget/udc/core.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/udc/core.c
b/drivers/usb/gadget/udc/core.c
index efce68e9a8e0..8155468afc0d 100644
--- a/drivers/usb/gadget/udc/core.c
+++ b/drivers/usb/gadget/udc/core.c
@@ -1234,8 +1234,12 @@ static void usb_gadget_remove_driver(struct
usb_udc *udc)

usb_gadget_disconnect(udc->gadget);
udc->driver->disconnect(udc->gadget);
-   udc->driver->unbind(udc->gadget);
+   /* udc_stop needs to be called before gadget driver unbind to
prevent
+* udc driver calls into gadget driver after unbind which could
cause
+* a nullptr exception.
+*/
usb_gadget_udc_stop(udc);
+   udc->driver->unbind(udc->gadget);


This patch is incorrect, it will prevent us from giving back requests
to
gadget driver properly. ->unbind() has to happen before ->udc_stop().


Do you mean after udc_stop the udc driver can not call the gadget 
driver

anymore? If not, I did not got your point, sorry for that. Can you
please
help me out? Would the changed order raise another issue I'm not aware
of?


right, ->udc_stop() is supposed to completely teardown the USB
controller, including disabling interrupts and all. The only thing it
_can_ do from ->udc_stop() would be giving back any pending requests
that were left (which would cause req->complete() to be called with an
error status). But even that is unlikely in the case you mention since
->unbind() was already called.


Ok, got it. That's why req->context = cdev, to overcome being unbound
already. Thanks for clarification.


If I understood you correctly, without this patch udc driver can not
call
the gadget driver back as well, because this would result in a NULL 
ptr

dereference, as unbind() sets drvdata to NULL.

In any case the race described in my original message can still 
happen,
regardless of the order of udc_stop and unbind. But with this patch 
the

needed locking could easily done within the udc driver only. Without,
the
lock needs to be acquired before udc->driver->unbind(udc->gadget) and
released after usb_gadget_udc_stop(). Otherwise an ISR of the udc 
driver
trying to call into the gadget driver could do this after gadget 
driver

already unbound.


right

Is someone working on this issue, already

Re: [PATCH] usb: gadget: udc: udc_stop before gadget unbind

[Danilo Krummrich]

On 2017-08-15 14:02, Felipe Balbi wrote:

Hi,

Danilo Krummrich  writes:

thanks for reviewing.


np :-)


On 2017-08-15 12:03, Felipe Balbi wrote:

Hi,

Danilo Krummrich  writes:
udc_stop needs to be called before gadget driver unbind. Otherwise 
it
might happen that udc drivers still call into the gadget driver 
(e.g.
to reset gadget after OTG event). If this happens it is likely to 
get
panics from gadget driver dereferencing NULL ptr, as gadget's 
drvdata

is set to NULL on unbind.


seems like the problem here is with the OTG layer, not UDC core.


I mentioned this just as example, it can happen whenever a UDC driver
calls
the gadget driver (e.g. by calling usb_gadget_udc_reset() in ISR) 
after

gadget
drivers unbind() was called already (e.g. by gadget configfs).
If this happens gadget drivers drvdata was already set to NULL by
unbind()
and reset() could result into a NULL ptr exception.
Therefore my assumption was that it needs to be prevented that the
gadget
driver is getting called after unbind.


We have a known problem in the design of the gadget API that causes 
this

races but we couldn't come up with a solution yet :-)

Inverting these two calls is not the correct way to go about this :-)


Now I see, thanks for explanation below.


Signed-off-by: Danilo Krummrich 
---
Actually there could still be a race:
(CPU1 code taken from dwc3 drivers dwc3_disconnect_gadget() as
exsample)

CPU0CPU1

usb_gadget_disconnect(udc->gadget);
udc->driver->disconnect(udc->gadget);
if (dwc->gadget_driver && 
dwc->gadget_driver->disconnect)
usb_gadget_udc_stop(udc);
udc->driver->unbind(udc->gadget);

dwc->gadget_driver->disconnect(>gadget);

UDC drivers typically set their gadget driver pointer to NULL in
udc_stop
and check for it before calling into the gadget driver. To fix the
issue
above every udc driver could apply a lock around this.

If you see the need for having this or another solutions I can 
provide

further patches. This patch could also just serve as a base for
discussion
if someone knows a smarter solution.

I saw this problem causing a panic on hikey960 board and provided a
quick
workaround for the same problem here:
https://android-review.googlesource.com/#/c/kernel/common/+/457476/
(panic log in the commit message of the linked patch)
---
 drivers/usb/gadget/udc/core.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/udc/core.c
b/drivers/usb/gadget/udc/core.c
index efce68e9a8e0..8155468afc0d 100644
--- a/drivers/usb/gadget/udc/core.c
+++ b/drivers/usb/gadget/udc/core.c
@@ -1234,8 +1234,12 @@ static void usb_gadget_remove_driver(struct
usb_udc *udc)

usb_gadget_disconnect(udc->gadget);
udc->driver->disconnect(udc->gadget);
-   udc->driver->unbind(udc->gadget);
+   /* udc_stop needs to be called before gadget driver unbind to
prevent
+* udc driver calls into gadget driver after unbind which could
cause
+* a nullptr exception.
+*/
usb_gadget_udc_stop(udc);
+   udc->driver->unbind(udc->gadget);


This patch is incorrect, it will prevent us from giving back requests
to
gadget driver properly. ->unbind() has to happen before ->udc_stop().


Do you mean after udc_stop the udc driver can not call the gadget 
driver

anymore? If not, I did not got your point, sorry for that. Can you
please
help me out? Would the changed order raise another issue I'm not aware
of?


right, ->udc_stop() is supposed to completely teardown the USB
controller, including disabling interrupts and all. The only thing it
_can_ do from ->udc_stop() would be giving back any pending requests
that were left (which would cause req->complete() to be called with an
error status). But even that is unlikely in the case you mention since
->unbind() was already called.


Ok, got it. That's why req->context = cdev, to overcome being unbound
already. Thanks for clarification.


If I understood you correctly, without this patch udc driver can not
call
the gadget driver back as well, because this would result in a NULL 
ptr

dereference, as unbind() sets drvdata to NULL.

In any case the race described in my original message can still 
happen,
regardless of the order of udc_stop and unbind. But with this patch 
the

needed locking could easily done within the udc driver only. Without,
the
lock needs to be acquired before udc->driver->unbind(udc->gadget) and
released after usb_gadget_udc_stop(). Otherwise an ISR of the udc 
driver
trying to call into the gadget driver could do this after gadget 
driver

already unbound.


right

Is someone working on this issue, already?
If not, I would like to offer introducing the needed locking to overcome
this issue.
If you are about to ref

Re: [PATCH] usb: gadget: udc: udc_stop before gadget unbind

[Danilo Krummrich]

Hi,

thanks for reviewing.

On 2017-08-15 12:03, Felipe Balbi wrote:

Hi,

Danilo Krummrich <danilokrummr...@dk-develop.de> writes:

udc_stop needs to be called before gadget driver unbind. Otherwise it
might happen that udc drivers still call into the gadget driver (e.g.
to reset gadget after OTG event). If this happens it is likely to get
panics from gadget driver dereferencing NULL ptr, as gadget's drvdata
is set to NULL on unbind.


seems like the problem here is with the OTG layer, not UDC core.

I mentioned this just as example, it can happen whenever a UDC driver 
calls
the gadget driver (e.g. by calling usb_gadget_udc_reset() in ISR) after 
gadget

drivers unbind() was called already (e.g. by gadget configfs).
If this happens gadget drivers drvdata was already set to NULL by 
unbind()

and reset() could result into a NULL ptr exception.
Therefore my assumption was that it needs to be prevented that the 
gadget

driver is getting called after unbind.

Signed-off-by: Danilo Krummrich <danilokrummr...@dk-develop.de>
---
Actually there could still be a race:
(CPU1 code taken from dwc3 drivers dwc3_disconnect_gadget() as 
exsample)


CPU0CPU1

usb_gadget_disconnect(udc->gadget);
udc->driver->disconnect(udc->gadget);
if (dwc->gadget_driver && 
dwc->gadget_driver->disconnect)
usb_gadget_udc_stop(udc);
udc->driver->unbind(udc->gadget);

dwc->gadget_driver->disconnect(>gadget);

UDC drivers typically set their gadget driver pointer to NULL in 
udc_stop
and check for it before calling into the gadget driver. To fix the 
issue

above every udc driver could apply a lock around this.

If you see the need for having this or another solutions I can provide
further patches. This patch could also just serve as a base for 
discussion

if someone knows a smarter solution.

I saw this problem causing a panic on hikey960 board and provided a 
quick

workaround for the same problem here:
https://android-review.googlesource.com/#/c/kernel/common/+/457476/
(panic log in the commit message of the linked patch)
---
 drivers/usb/gadget/udc/core.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/udc/core.c 
b/drivers/usb/gadget/udc/core.c

index efce68e9a8e0..8155468afc0d 100644
--- a/drivers/usb/gadget/udc/core.c
+++ b/drivers/usb/gadget/udc/core.c
@@ -1234,8 +1234,12 @@ static void usb_gadget_remove_driver(struct 
usb_udc *udc)


usb_gadget_disconnect(udc->gadget);
udc->driver->disconnect(udc->gadget);
-   udc->driver->unbind(udc->gadget);
+	/* udc_stop needs to be called before gadget driver unbind to 
prevent
+	 * udc driver calls into gadget driver after unbind which could 
cause

+* a nullptr exception.
+*/
usb_gadget_udc_stop(udc);
+   udc->driver->unbind(udc->gadget);


This patch is incorrect, it will prevent us from giving back requests 
to

gadget driver properly. ->unbind() has to happen before ->udc_stop().


Do you mean after udc_stop the udc driver can not call the gadget driver
anymore? If not, I did not got your point, sorry for that. Can you 
please
help me out? Would the changed order raise another issue I'm not aware 
of?


If I understood you correctly, without this patch udc driver can not 
call

the gadget driver back as well, because this would result in a NULL ptr
dereference, as unbind() sets drvdata to NULL.

In any case the race described in my original message can still happen,
regardless of the order of udc_stop and unbind. But with this patch the
needed locking could easily done within the udc driver only. Without, 
the

lock needs to be acquired before udc->driver->unbind(udc->gadget) and
released after usb_gadget_udc_stop(). Otherwise an ISR of the udc driver
trying to call into the gadget driver could do this after gadget driver
already unbound.

Regards,
Danilo

Re: [PATCH] usb: gadget: udc: udc_stop before gadget unbind

[Danilo Krummrich]

Hi,

thanks for reviewing.

On 2017-08-15 12:03, Felipe Balbi wrote:

Hi,

Danilo Krummrich  writes:

udc_stop needs to be called before gadget driver unbind. Otherwise it
might happen that udc drivers still call into the gadget driver (e.g.
to reset gadget after OTG event). If this happens it is likely to get
panics from gadget driver dereferencing NULL ptr, as gadget's drvdata
is set to NULL on unbind.


seems like the problem here is with the OTG layer, not UDC core.

I mentioned this just as example, it can happen whenever a UDC driver 
calls
the gadget driver (e.g. by calling usb_gadget_udc_reset() in ISR) after 
gadget

drivers unbind() was called already (e.g. by gadget configfs).
If this happens gadget drivers drvdata was already set to NULL by 
unbind()

and reset() could result into a NULL ptr exception.
Therefore my assumption was that it needs to be prevented that the 
gadget

driver is getting called after unbind.

Signed-off-by: Danilo Krummrich 
---
Actually there could still be a race:
(CPU1 code taken from dwc3 drivers dwc3_disconnect_gadget() as 
exsample)


CPU0CPU1

usb_gadget_disconnect(udc->gadget);
udc->driver->disconnect(udc->gadget);
if (dwc->gadget_driver && 
dwc->gadget_driver->disconnect)
usb_gadget_udc_stop(udc);
udc->driver->unbind(udc->gadget);

dwc->gadget_driver->disconnect(>gadget);

UDC drivers typically set their gadget driver pointer to NULL in 
udc_stop
and check for it before calling into the gadget driver. To fix the 
issue

above every udc driver could apply a lock around this.

If you see the need for having this or another solutions I can provide
further patches. This patch could also just serve as a base for 
discussion

if someone knows a smarter solution.

I saw this problem causing a panic on hikey960 board and provided a 
quick

workaround for the same problem here:
https://android-review.googlesource.com/#/c/kernel/common/+/457476/
(panic log in the commit message of the linked patch)
---
 drivers/usb/gadget/udc/core.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/udc/core.c 
b/drivers/usb/gadget/udc/core.c

index efce68e9a8e0..8155468afc0d 100644
--- a/drivers/usb/gadget/udc/core.c
+++ b/drivers/usb/gadget/udc/core.c
@@ -1234,8 +1234,12 @@ static void usb_gadget_remove_driver(struct 
usb_udc *udc)


usb_gadget_disconnect(udc->gadget);
udc->driver->disconnect(udc->gadget);
-   udc->driver->unbind(udc->gadget);
+	/* udc_stop needs to be called before gadget driver unbind to 
prevent
+	 * udc driver calls into gadget driver after unbind which could 
cause

+* a nullptr exception.
+*/
usb_gadget_udc_stop(udc);
+   udc->driver->unbind(udc->gadget);


This patch is incorrect, it will prevent us from giving back requests 
to

gadget driver properly. ->unbind() has to happen before ->udc_stop().


Do you mean after udc_stop the udc driver can not call the gadget driver
anymore? If not, I did not got your point, sorry for that. Can you 
please
help me out? Would the changed order raise another issue I'm not aware 
of?


If I understood you correctly, without this patch udc driver can not 
call

the gadget driver back as well, because this would result in a NULL ptr
dereference, as unbind() sets drvdata to NULL.

In any case the race described in my original message can still happen,
regardless of the order of udc_stop and unbind. But with this patch the
needed locking could easily done within the udc driver only. Without, 
the

lock needs to be acquired before udc->driver->unbind(udc->gadget) and
released after usb_gadget_udc_stop(). Otherwise an ISR of the udc driver
trying to call into the gadget driver could do this after gadget driver
already unbound.

Regards,
Danilo

[PATCH] usb: gadget: udc: udc_stop before gadget unbind

[Danilo Krummrich]

udc_stop needs to be called before gadget driver unbind. Otherwise it
might happen that udc drivers still call into the gadget driver (e.g.
to reset gadget after OTG event). If this happens it is likely to get
panics from gadget driver dereferencing NULL ptr, as gadget's drvdata
is set to NULL on unbind.

Signed-off-by: Danilo Krummrich <danilokrummr...@dk-develop.de>
---
Actually there could still be a race:
(CPU1 code taken from dwc3 drivers dwc3_disconnect_gadget() as exsample)

CPU0CPU1

usb_gadget_disconnect(udc->gadget);
udc->driver->disconnect(udc->gadget);
if (dwc->gadget_driver && 
dwc->gadget_driver->disconnect)
usb_gadget_udc_stop(udc);
udc->driver->unbind(udc->gadget);

dwc->gadget_driver->disconnect(>gadget);

UDC drivers typically set their gadget driver pointer to NULL in udc_stop
and check for it before calling into the gadget driver. To fix the issue
above every udc driver could apply a lock around this.

If you see the need for having this or another solutions I can provide
further patches. This patch could also just serve as a base for discussion
if someone knows a smarter solution.

I saw this problem causing a panic on hikey960 board and provided a quick
workaround for the same problem here:
https://android-review.googlesource.com/#/c/kernel/common/+/457476/
(panic log in the commit message of the linked patch)
---
 drivers/usb/gadget/udc/core.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c
index efce68e9a8e0..8155468afc0d 100644
--- a/drivers/usb/gadget/udc/core.c
+++ b/drivers/usb/gadget/udc/core.c
@@ -1234,8 +1234,12 @@ static void usb_gadget_remove_driver(struct usb_udc *udc)
 
usb_gadget_disconnect(udc->gadget);
udc->driver->disconnect(udc->gadget);
-   udc->driver->unbind(udc->gadget);
+   /* udc_stop needs to be called before gadget driver unbind to prevent
+* udc driver calls into gadget driver after unbind which could cause
+* a nullptr exception.
+*/
usb_gadget_udc_stop(udc);
+   udc->driver->unbind(udc->gadget);
 
udc->driver = NULL;
udc->dev.driver = NULL;
-- 
2.14.1

[PATCH] usb: gadget: udc: udc_stop before gadget unbind

[Danilo Krummrich]

udc_stop needs to be called before gadget driver unbind. Otherwise it
might happen that udc drivers still call into the gadget driver (e.g.
to reset gadget after OTG event). If this happens it is likely to get
panics from gadget driver dereferencing NULL ptr, as gadget's drvdata
is set to NULL on unbind.

Signed-off-by: Danilo Krummrich 
---
Actually there could still be a race:
(CPU1 code taken from dwc3 drivers dwc3_disconnect_gadget() as exsample)

CPU0CPU1

usb_gadget_disconnect(udc->gadget);
udc->driver->disconnect(udc->gadget);
if (dwc->gadget_driver && 
dwc->gadget_driver->disconnect)
usb_gadget_udc_stop(udc);
udc->driver->unbind(udc->gadget);

dwc->gadget_driver->disconnect(>gadget);

UDC drivers typically set their gadget driver pointer to NULL in udc_stop
and check for it before calling into the gadget driver. To fix the issue
above every udc driver could apply a lock around this.

If you see the need for having this or another solutions I can provide
further patches. This patch could also just serve as a base for discussion
if someone knows a smarter solution.

I saw this problem causing a panic on hikey960 board and provided a quick
workaround for the same problem here:
https://android-review.googlesource.com/#/c/kernel/common/+/457476/
(panic log in the commit message of the linked patch)
---
 drivers/usb/gadget/udc/core.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c
index efce68e9a8e0..8155468afc0d 100644
--- a/drivers/usb/gadget/udc/core.c
+++ b/drivers/usb/gadget/udc/core.c
@@ -1234,8 +1234,12 @@ static void usb_gadget_remove_driver(struct usb_udc *udc)
 
usb_gadget_disconnect(udc->gadget);
udc->driver->disconnect(udc->gadget);
-   udc->driver->unbind(udc->gadget);
+   /* udc_stop needs to be called before gadget driver unbind to prevent
+* udc driver calls into gadget driver after unbind which could cause
+* a nullptr exception.
+*/
usb_gadget_udc_stop(udc);
+   udc->driver->unbind(udc->gadget);
 
udc->driver = NULL;
udc->dev.driver = NULL;
-- 
2.14.1

[PATCH v7] serio: PS/2 gpio bit banging driver for serio bus

[Danilo Krummrich]

This driver provides PS/2 serio bus support by implementing bit banging
with the GPIO API. The GPIO pins, data and clock, can be configured with
a node in the device tree or by generic device properties (GDP).

Writing to a device is supported as well, though it is possible timings
can not be halt as they are tough and difficult to reach with bit banging.
Therefore it can be configured (also in DT and GDP) whether the serio
write function should be available for clients.

This driver is for development purposes and not recommended for productive
use. However, this driver can be useful e.g. when no USB port is available
or using old peripherals is desired as PS/2 controller chips getting rare.

This driver was tested on RPI1 and on Hikey960 and it worked well together
with the atkbd and psmouse driver.

Signed-off-by: Danilo Krummrich <danilokrummr...@dk-develop.de>
---
v2: Removed one verbose print statement, changed another one to dev_dbg.
v3: - fixed compiler warning on blackfin
- depends on GPIOLIB
- clarify documentation
v4: - fixed concurrent calls to ps2_gpio_write (serio->write)
- use gpiod API
- use generic device properties
- request irq separately, do not use gpiod_to_irq
- abort when gpio is connected via slow bus
- Fixed a bug where PS2_CMD_RESEND is always send after tx failed once.
  The makes the write functionallity work better, tough timing is still
  critical.
- disable irq initially until ps2_gpip_open (serio->open) is called
v5: Checked again why timings are that hard to reach while in tx mode and
discovered that there is an extra clock pulse between stop bit sent from
host and acknowledgement from device. By just skipping this clock pulse
tx works fine now, though it still happens sometimes that the timing can
not be reached of course.
v6: - fixed typos
- use of_match_ptr
v7: remove unnecessary barriers

Sorry for resending, forgot der version tag in the subject.
---
 .../devicetree/bindings/serio/ps2-gpio.txt |  22 +
 Documentation/gpio/drivers-on-gpio.txt |   5 +
 drivers/input/serio/Kconfig|  11 +
 drivers/input/serio/Makefile   |   1 +
 drivers/input/serio/ps2-gpio.c | 453 +
 5 files changed, 492 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/serio/ps2-gpio.txt
 create mode 100644 drivers/input/serio/ps2-gpio.c

diff --git a/Documentation/devicetree/bindings/serio/ps2-gpio.txt 
b/Documentation/devicetree/bindings/serio/ps2-gpio.txt
new file mode 100644
index ..099dd6d46cb3
--- /dev/null
+++ b/Documentation/devicetree/bindings/serio/ps2-gpio.txt
@@ -0,0 +1,22 @@
+Device-Tree bindings for ps/2 gpio driver
+
+Required properties:
+   - compatible = "ps2-gpio"
+   - gpios: data and clock gpio
+   - interrupts: Should trigger on the falling edge of the clock line.
+
+Optional properties:
+   - ps2-gpio,write-enable: Indicates whether write function is provided
+   to serio device. Possibly providing the write fn will not work, because
+   of the tough timing requirements.
+
+Example nodes:
+
+ps2@0 {
+   compatible = "ps2-gpio";
+   interrupt-parent = <>;
+   interrupts = <23 IRQ_TYPE_EDGE_FALLING>;
+   data-gpios = < 24 GPIO_ACTIVE_HIGH>;
+   clk-gpios = < 23 GPIO_ACTIVE_HIGH>;
+   ps2-gpio,write-enable;
+};
diff --git a/Documentation/gpio/drivers-on-gpio.txt 
b/Documentation/gpio/drivers-on-gpio.txt
index 306513251713..9a78d385b92e 100644
--- a/Documentation/gpio/drivers-on-gpio.txt
+++ b/Documentation/gpio/drivers-on-gpio.txt
@@ -84,6 +84,11 @@ hardware descriptions such as device tree or ACPI:
   NAND flash MTD subsystem and provides chip access and partition parsing like
   any other NAND driving hardware.
 
+- ps2-gpio: drivers/input/serio/ps2-gpio.c is used to drive a PS/2 (IBM) serio
+  bus, data and clock line, by bit banging two GPIO lines. It will appear as
+  any other serio bus to the system and makes it possible to connect drivers
+  for e.g. keyboards and other PS/2 protocol based devices.
+
 Apart from this there are special GPIO drivers in subsystems like MMC/SD to
 read card detect and write protect GPIO lines, and in the TTY serial subsystem
 to emulate MCTRL (modem control) signals CTS/RTS by using two GPIO lines. The
diff --git a/drivers/input/serio/Kconfig b/drivers/input/serio/Kconfig
index c3d05b4d3118..21488c048fa3 100644
--- a/drivers/input/serio/Kconfig
+++ b/drivers/input/serio/Kconfig
@@ -292,6 +292,17 @@ config SERIO_SUN4I_PS2
  To compile this driver as a module, choose M here: the
  module will be called sun4i-ps2.
 
+config SERIO_GPIO_PS2
+   tristate "GPIO PS/2 bit banging driver"
+   depends on GPIOLIB
+   help
+ Say Y here if you want PS/2 bit banging support via GPIO.
+
+ To compile this driver as a m

[PATCH v7] serio: PS/2 gpio bit banging driver for serio bus

[Danilo Krummrich]

This driver provides PS/2 serio bus support by implementing bit banging
with the GPIO API. The GPIO pins, data and clock, can be configured with
a node in the device tree or by generic device properties (GDP).

Writing to a device is supported as well, though it is possible timings
can not be halt as they are tough and difficult to reach with bit banging.
Therefore it can be configured (also in DT and GDP) whether the serio
write function should be available for clients.

This driver is for development purposes and not recommended for productive
use. However, this driver can be useful e.g. when no USB port is available
or using old peripherals is desired as PS/2 controller chips getting rare.

This driver was tested on RPI1 and on Hikey960 and it worked well together
with the atkbd and psmouse driver.

Signed-off-by: Danilo Krummrich 
---
v2: Removed one verbose print statement, changed another one to dev_dbg.
v3: - fixed compiler warning on blackfin
- depends on GPIOLIB
- clarify documentation
v4: - fixed concurrent calls to ps2_gpio_write (serio->write)
- use gpiod API
- use generic device properties
- request irq separately, do not use gpiod_to_irq
- abort when gpio is connected via slow bus
- Fixed a bug where PS2_CMD_RESEND is always send after tx failed once.
  The makes the write functionallity work better, tough timing is still
  critical.
- disable irq initially until ps2_gpip_open (serio->open) is called
v5: Checked again why timings are that hard to reach while in tx mode and
discovered that there is an extra clock pulse between stop bit sent from
host and acknowledgement from device. By just skipping this clock pulse
tx works fine now, though it still happens sometimes that the timing can
not be reached of course.
v6: - fixed typos
- use of_match_ptr
v7: remove unnecessary barriers

Sorry for resending, forgot der version tag in the subject.
---
 .../devicetree/bindings/serio/ps2-gpio.txt |  22 +
 Documentation/gpio/drivers-on-gpio.txt |   5 +
 drivers/input/serio/Kconfig|  11 +
 drivers/input/serio/Makefile   |   1 +
 drivers/input/serio/ps2-gpio.c | 453 +
 5 files changed, 492 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/serio/ps2-gpio.txt
 create mode 100644 drivers/input/serio/ps2-gpio.c

diff --git a/Documentation/devicetree/bindings/serio/ps2-gpio.txt 
b/Documentation/devicetree/bindings/serio/ps2-gpio.txt
new file mode 100644
index ..099dd6d46cb3
--- /dev/null
+++ b/Documentation/devicetree/bindings/serio/ps2-gpio.txt
@@ -0,0 +1,22 @@
+Device-Tree bindings for ps/2 gpio driver
+
+Required properties:
+   - compatible = "ps2-gpio"
+   - gpios: data and clock gpio
+   - interrupts: Should trigger on the falling edge of the clock line.
+
+Optional properties:
+   - ps2-gpio,write-enable: Indicates whether write function is provided
+   to serio device. Possibly providing the write fn will not work, because
+   of the tough timing requirements.
+
+Example nodes:
+
+ps2@0 {
+   compatible = "ps2-gpio";
+   interrupt-parent = <>;
+   interrupts = <23 IRQ_TYPE_EDGE_FALLING>;
+   data-gpios = < 24 GPIO_ACTIVE_HIGH>;
+   clk-gpios = < 23 GPIO_ACTIVE_HIGH>;
+   ps2-gpio,write-enable;
+};
diff --git a/Documentation/gpio/drivers-on-gpio.txt 
b/Documentation/gpio/drivers-on-gpio.txt
index 306513251713..9a78d385b92e 100644
--- a/Documentation/gpio/drivers-on-gpio.txt
+++ b/Documentation/gpio/drivers-on-gpio.txt
@@ -84,6 +84,11 @@ hardware descriptions such as device tree or ACPI:
   NAND flash MTD subsystem and provides chip access and partition parsing like
   any other NAND driving hardware.
 
+- ps2-gpio: drivers/input/serio/ps2-gpio.c is used to drive a PS/2 (IBM) serio
+  bus, data and clock line, by bit banging two GPIO lines. It will appear as
+  any other serio bus to the system and makes it possible to connect drivers
+  for e.g. keyboards and other PS/2 protocol based devices.
+
 Apart from this there are special GPIO drivers in subsystems like MMC/SD to
 read card detect and write protect GPIO lines, and in the TTY serial subsystem
 to emulate MCTRL (modem control) signals CTS/RTS by using two GPIO lines. The
diff --git a/drivers/input/serio/Kconfig b/drivers/input/serio/Kconfig
index c3d05b4d3118..21488c048fa3 100644
--- a/drivers/input/serio/Kconfig
+++ b/drivers/input/serio/Kconfig
@@ -292,6 +292,17 @@ config SERIO_SUN4I_PS2
  To compile this driver as a module, choose M here: the
  module will be called sun4i-ps2.
 
+config SERIO_GPIO_PS2
+   tristate "GPIO PS/2 bit banging driver"
+   depends on GPIOLIB
+   help
+ Say Y here if you want PS/2 bit banging support via GPIO.
+
+ To compile this driver as a module, choose M here: the
+ m

[PATCH] serio: PS/2 gpio bit banging driver for serio bus

[Danilo Krummrich]

This driver provides PS/2 serio bus support by implementing bit banging
with the GPIO API. The GPIO pins, data and clock, can be configured with
a node in the device tree or by generic device properties (GDP).

Writing to a device is supported as well, though it is possible timings
can not be halt as they are tough and difficult to reach with bit banging.
Therefore it can be configured (also in DT and GDP) whether the serio
write function should be available for clients.

This driver is for development purposes and not recommended for productive
use. However, this driver can be useful e.g. when no USB port is available
or using old peripherals is desired as PS/2 controller chips getting rare.

This driver was tested on RPI1 and on Hikey960 and it worked well together
with the atkbd and psmouse driver.

Signed-off-by: Danilo Krummrich <danilokrummr...@dk-develop.de>
---
v2: Removed one verbose print statement, changed another one to dev_dbg.
v3: - fixed compiler warning on blackfin
- depends on GPIOLIB
- clarify documentation
v4: - fixed concurrent calls to ps2_gpio_write (serio->write)
- use gpiod API
- use generic device properties
- request irq separately, do not use gpiod_to_irq
- abort when gpio is connected via slow bus
- Fixed a bug where PS2_CMD_RESEND is always send after tx failed once.
  The makes the write functionallity work better, tough timing is still
  critical.
- disable irq initially until ps2_gpip_open (serio->open) is called
v5: Checked again why timings are that hard to reach while in tx mode and
discovered that there is an extra clock pulse between stop bit sent from
host and acknowledgement from device. By just skipping this clock pulse
tx works fine now, though it still happens sometimes that the timing can
not be reached of course.
v6: - fixed typos
- use of_match_ptr
v7: remove unnecessary barriers
---
 .../devicetree/bindings/serio/ps2-gpio.txt |  22 +
 Documentation/gpio/drivers-on-gpio.txt |   5 +
 drivers/input/serio/Kconfig|  11 +
 drivers/input/serio/Makefile   |   1 +
 drivers/input/serio/ps2-gpio.c | 453 +
 5 files changed, 492 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/serio/ps2-gpio.txt
 create mode 100644 drivers/input/serio/ps2-gpio.c

diff --git a/Documentation/devicetree/bindings/serio/ps2-gpio.txt 
b/Documentation/devicetree/bindings/serio/ps2-gpio.txt
new file mode 100644
index ..099dd6d46cb3
--- /dev/null
+++ b/Documentation/devicetree/bindings/serio/ps2-gpio.txt
@@ -0,0 +1,22 @@
+Device-Tree bindings for ps/2 gpio driver
+
+Required properties:
+   - compatible = "ps2-gpio"
+   - gpios: data and clock gpio
+   - interrupts: Should trigger on the falling edge of the clock line.
+
+Optional properties:
+   - ps2-gpio,write-enable: Indicates whether write function is provided
+   to serio device. Possibly providing the write fn will not work, because
+   of the tough timing requirements.
+
+Example nodes:
+
+ps2@0 {
+   compatible = "ps2-gpio";
+   interrupt-parent = <>;
+   interrupts = <23 IRQ_TYPE_EDGE_FALLING>;
+   data-gpios = < 24 GPIO_ACTIVE_HIGH>;
+   clk-gpios = < 23 GPIO_ACTIVE_HIGH>;
+   ps2-gpio,write-enable;
+};
diff --git a/Documentation/gpio/drivers-on-gpio.txt 
b/Documentation/gpio/drivers-on-gpio.txt
index 306513251713..9a78d385b92e 100644
--- a/Documentation/gpio/drivers-on-gpio.txt
+++ b/Documentation/gpio/drivers-on-gpio.txt
@@ -84,6 +84,11 @@ hardware descriptions such as device tree or ACPI:
   NAND flash MTD subsystem and provides chip access and partition parsing like
   any other NAND driving hardware.
 
+- ps2-gpio: drivers/input/serio/ps2-gpio.c is used to drive a PS/2 (IBM) serio
+  bus, data and clock line, by bit banging two GPIO lines. It will appear as
+  any other serio bus to the system and makes it possible to connect drivers
+  for e.g. keyboards and other PS/2 protocol based devices.
+
 Apart from this there are special GPIO drivers in subsystems like MMC/SD to
 read card detect and write protect GPIO lines, and in the TTY serial subsystem
 to emulate MCTRL (modem control) signals CTS/RTS by using two GPIO lines. The
diff --git a/drivers/input/serio/Kconfig b/drivers/input/serio/Kconfig
index c3d05b4d3118..21488c048fa3 100644
--- a/drivers/input/serio/Kconfig
+++ b/drivers/input/serio/Kconfig
@@ -292,6 +292,17 @@ config SERIO_SUN4I_PS2
  To compile this driver as a module, choose M here: the
  module will be called sun4i-ps2.
 
+config SERIO_GPIO_PS2
+   tristate "GPIO PS/2 bit banging driver"
+   depends on GPIOLIB
+   help
+ Say Y here if you want PS/2 bit banging support via GPIO.
+
+ To compile this driver as a module, choose M here: the
+ module will be calle

[PATCH] serio: PS/2 gpio bit banging driver for serio bus

[Danilo Krummrich]

This driver provides PS/2 serio bus support by implementing bit banging
with the GPIO API. The GPIO pins, data and clock, can be configured with
a node in the device tree or by generic device properties (GDP).

Writing to a device is supported as well, though it is possible timings
can not be halt as they are tough and difficult to reach with bit banging.
Therefore it can be configured (also in DT and GDP) whether the serio
write function should be available for clients.

This driver is for development purposes and not recommended for productive
use. However, this driver can be useful e.g. when no USB port is available
or using old peripherals is desired as PS/2 controller chips getting rare.

This driver was tested on RPI1 and on Hikey960 and it worked well together
with the atkbd and psmouse driver.

Signed-off-by: Danilo Krummrich 
---
v2: Removed one verbose print statement, changed another one to dev_dbg.
v3: - fixed compiler warning on blackfin
- depends on GPIOLIB
- clarify documentation
v4: - fixed concurrent calls to ps2_gpio_write (serio->write)
- use gpiod API
- use generic device properties
- request irq separately, do not use gpiod_to_irq
- abort when gpio is connected via slow bus
- Fixed a bug where PS2_CMD_RESEND is always send after tx failed once.
  The makes the write functionallity work better, tough timing is still
  critical.
- disable irq initially until ps2_gpip_open (serio->open) is called
v5: Checked again why timings are that hard to reach while in tx mode and
discovered that there is an extra clock pulse between stop bit sent from
host and acknowledgement from device. By just skipping this clock pulse
tx works fine now, though it still happens sometimes that the timing can
not be reached of course.
v6: - fixed typos
- use of_match_ptr
v7: remove unnecessary barriers
---
 .../devicetree/bindings/serio/ps2-gpio.txt |  22 +
 Documentation/gpio/drivers-on-gpio.txt |   5 +
 drivers/input/serio/Kconfig|  11 +
 drivers/input/serio/Makefile   |   1 +
 drivers/input/serio/ps2-gpio.c | 453 +
 5 files changed, 492 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/serio/ps2-gpio.txt
 create mode 100644 drivers/input/serio/ps2-gpio.c

diff --git a/Documentation/devicetree/bindings/serio/ps2-gpio.txt 
b/Documentation/devicetree/bindings/serio/ps2-gpio.txt
new file mode 100644
index ..099dd6d46cb3
--- /dev/null
+++ b/Documentation/devicetree/bindings/serio/ps2-gpio.txt
@@ -0,0 +1,22 @@
+Device-Tree bindings for ps/2 gpio driver
+
+Required properties:
+   - compatible = "ps2-gpio"
+   - gpios: data and clock gpio
+   - interrupts: Should trigger on the falling edge of the clock line.
+
+Optional properties:
+   - ps2-gpio,write-enable: Indicates whether write function is provided
+   to serio device. Possibly providing the write fn will not work, because
+   of the tough timing requirements.
+
+Example nodes:
+
+ps2@0 {
+   compatible = "ps2-gpio";
+   interrupt-parent = <>;
+   interrupts = <23 IRQ_TYPE_EDGE_FALLING>;
+   data-gpios = < 24 GPIO_ACTIVE_HIGH>;
+   clk-gpios = < 23 GPIO_ACTIVE_HIGH>;
+   ps2-gpio,write-enable;
+};
diff --git a/Documentation/gpio/drivers-on-gpio.txt 
b/Documentation/gpio/drivers-on-gpio.txt
index 306513251713..9a78d385b92e 100644
--- a/Documentation/gpio/drivers-on-gpio.txt
+++ b/Documentation/gpio/drivers-on-gpio.txt
@@ -84,6 +84,11 @@ hardware descriptions such as device tree or ACPI:
   NAND flash MTD subsystem and provides chip access and partition parsing like
   any other NAND driving hardware.
 
+- ps2-gpio: drivers/input/serio/ps2-gpio.c is used to drive a PS/2 (IBM) serio
+  bus, data and clock line, by bit banging two GPIO lines. It will appear as
+  any other serio bus to the system and makes it possible to connect drivers
+  for e.g. keyboards and other PS/2 protocol based devices.
+
 Apart from this there are special GPIO drivers in subsystems like MMC/SD to
 read card detect and write protect GPIO lines, and in the TTY serial subsystem
 to emulate MCTRL (modem control) signals CTS/RTS by using two GPIO lines. The
diff --git a/drivers/input/serio/Kconfig b/drivers/input/serio/Kconfig
index c3d05b4d3118..21488c048fa3 100644
--- a/drivers/input/serio/Kconfig
+++ b/drivers/input/serio/Kconfig
@@ -292,6 +292,17 @@ config SERIO_SUN4I_PS2
  To compile this driver as a module, choose M here: the
  module will be called sun4i-ps2.
 
+config SERIO_GPIO_PS2
+   tristate "GPIO PS/2 bit banging driver"
+   depends on GPIOLIB
+   help
+ Say Y here if you want PS/2 bit banging support via GPIO.
+
+ To compile this driver as a module, choose M here: the
+ module will be called ps2-gpio.
+
+ If you are unsure

Re: [PATCH] serio: PS2 gpio bit banging driver for the serio bus

[Danilo Krummrich]

On 2017-08-11 11:16, Linus Walleij wrote:

On Thu, Aug 10, 2017 at 4:38 PM, Danilo Krummrich
<danilokrummr...@dk-develop.de> wrote:

On 2017-08-07 18:22, Danilo Krummrich wrote:


> +static int ps2_gpio_write(struct serio *serio, unsigned char val)
> +{
> +   struct ps2_gpio_data *drvdata = serio->port_data;
> +
> +   drvdata->mode = PS2_MODE_TX;
> +   drvdata->tx_byte = val;
> +   /* Make sure ISR running on other CPU notice changes. */
> +   barrier();

This seems overengineered, is this really needed?

If we have races like this, the error is likely elsewhere, and 
should be

fixed in the GPIO driver MMIO access or so.

Yes, seems it can be removed. I didn't saw any explicit barriers in 
the

GPIO
driver (I'm testing on bcm2835), but it seems MMIO operations on SMP 
archs
does contain barriers. Not sure if all do. If some do not this 
barrier

might
be needed to ensure ISR on other CPU notice the correct mode and byte 
to

send.

I couldn't find any guarantee that the mode and tx_byte change is 
implicitly
covered by a barrier in this case. E.g. the bcm2835 driver does not 
make
sure stores are completed before the particular interrupt is enabled, 
except by
the fact that writel on ARM contains a wmb(). But this is nothing to 
rely on.

(Please tell me if I miss something.)


writel() should be guaranteeing that the values hit the hardware, wmb() 
is

spelled out "write memory barrier" I don't see what you're after here.

Sorry for confusing wording. What I actually meant is if writel() is 
guaranteed
to make sure there's no reordering happening with other store 
operations. Of
course, in case of ARM it is sufficient as it contains a wmb. But I 
wasn't aware

that all writel() implementations guarantee this (if needed).
Thanks for clarification.

If you think writel() doesn't do its job on some platform, then fix 
writel()

on that platform.

We can't randomly sprinkle things like this all over the kernel it 
makes

no sense.

Therefore I would like to keep this barrier and replace it with 
smp_wmb() if

you are fine with that.


I do not think this is proper.

As you explained writel() should guarantee no reordering with other 
store operations
(like drvdata->mode = PS2_MODE_TX in my case) is happening, I totally 
agree and will

fix this.


Yours,
Linus Walleij


Thanks,
Danilo

Re: [PATCH] serio: PS2 gpio bit banging driver for the serio bus

[Danilo Krummrich]

On 2017-08-11 11:16, Linus Walleij wrote:

On Thu, Aug 10, 2017 at 4:38 PM, Danilo Krummrich
 wrote:

On 2017-08-07 18:22, Danilo Krummrich wrote:


> +static int ps2_gpio_write(struct serio *serio, unsigned char val)
> +{
> +   struct ps2_gpio_data *drvdata = serio->port_data;
> +
> +   drvdata->mode = PS2_MODE_TX;
> +   drvdata->tx_byte = val;
> +   /* Make sure ISR running on other CPU notice changes. */
> +   barrier();

This seems overengineered, is this really needed?

If we have races like this, the error is likely elsewhere, and 
should be

fixed in the GPIO driver MMIO access or so.

Yes, seems it can be removed. I didn't saw any explicit barriers in 
the

GPIO
driver (I'm testing on bcm2835), but it seems MMIO operations on SMP 
archs
does contain barriers. Not sure if all do. If some do not this 
barrier

might
be needed to ensure ISR on other CPU notice the correct mode and byte 
to

send.

I couldn't find any guarantee that the mode and tx_byte change is 
implicitly
covered by a barrier in this case. E.g. the bcm2835 driver does not 
make
sure stores are completed before the particular interrupt is enabled, 
except by
the fact that writel on ARM contains a wmb(). But this is nothing to 
rely on.

(Please tell me if I miss something.)


writel() should be guaranteeing that the values hit the hardware, wmb() 
is

spelled out "write memory barrier" I don't see what you're after here.

Sorry for confusing wording. What I actually meant is if writel() is 
guaranteed
to make sure there's no reordering happening with other store 
operations. Of
course, in case of ARM it is sufficient as it contains a wmb. But I 
wasn't aware

that all writel() implementations guarantee this (if needed).
Thanks for clarification.

If you think writel() doesn't do its job on some platform, then fix 
writel()

on that platform.

We can't randomly sprinkle things like this all over the kernel it 
makes

no sense.

Therefore I would like to keep this barrier and replace it with 
smp_wmb() if

you are fine with that.


I do not think this is proper.

As you explained writel() should guarantee no reordering with other 
store operations
(like drvdata->mode = PS2_MODE_TX in my case) is happening, I totally 
agree and will

fix this.


Yours,
Linus Walleij


Thanks,
Danilo

Re: [PATCH] serio: PS2 gpio bit banging driver for the serio bus

[Danilo Krummrich]

Hi Linus,

On 2017-08-07 18:22, Danilo Krummrich wrote:

> +static int ps2_gpio_write(struct serio *serio, unsigned char val)
> +{
> +   struct ps2_gpio_data *drvdata = serio->port_data;
> +
> +   drvdata->mode = PS2_MODE_TX;
> +   drvdata->tx_byte = val;
> +   /* Make sure ISR running on other CPU notice changes. */
> +   barrier();

This seems overengineered, is this really needed?

If we have races like this, the error is likely elsewhere, and should 
be

fixed in the GPIO driver MMIO access or so.

Yes, seems it can be removed. I didn't saw any explicit barriers in the 
GPIO
driver (I'm testing on bcm2835), but it seems MMIO operations on SMP 
archs
does contain barriers. Not sure if all do. If some do not this barrier 
might
be needed to ensure ISR on other CPU notice the correct mode and byte 
to send.


I couldn't find any guarantee that the mode and tx_byte change is 
implicitly
covered by a barrier in this case. E.g. the bcm2835 driver does not make 
sure
stores are completed before the particular interrupt is enabled, except 
by the
fact that writel on ARM contains a wmb(). But this is nothing to rely 
on. (Please

tell me if I miss something.)
Therefore I would like to keep this barrier and replace it with 
smp_wmb() if you

are fine with that.

Regards,
Danilo

Re: [PATCH] serio: PS2 gpio bit banging driver for the serio bus

[Danilo Krummrich]

Hi Linus,

On 2017-08-07 18:22, Danilo Krummrich wrote:

> +static int ps2_gpio_write(struct serio *serio, unsigned char val)
> +{
> +   struct ps2_gpio_data *drvdata = serio->port_data;
> +
> +   drvdata->mode = PS2_MODE_TX;
> +   drvdata->tx_byte = val;
> +   /* Make sure ISR running on other CPU notice changes. */
> +   barrier();

This seems overengineered, is this really needed?

If we have races like this, the error is likely elsewhere, and should 
be

fixed in the GPIO driver MMIO access or so.

Yes, seems it can be removed. I didn't saw any explicit barriers in the 
GPIO
driver (I'm testing on bcm2835), but it seems MMIO operations on SMP 
archs
does contain barriers. Not sure if all do. If some do not this barrier 
might
be needed to ensure ISR on other CPU notice the correct mode and byte 
to send.


I couldn't find any guarantee that the mode and tx_byte change is 
implicitly
covered by a barrier in this case. E.g. the bcm2835 driver does not make 
sure
stores are completed before the particular interrupt is enabled, except 
by the
fact that writel on ARM contains a wmb(). But this is nothing to rely 
on. (Please

tell me if I miss something.)
Therefore I would like to keep this barrier and replace it with 
smp_wmb() if you

are fine with that.

Regards,
Danilo

Re: [PATCH] serio: PS2 gpio bit banging driver for the serio bus

[Danilo Krummrich]

Hi Linus,

thanks for reviewing. Commented the ones which still holds.

On Mon, 7 Aug 2017 11:03:53 +0200
Linus Walleij <linus.wall...@linaro.org> wrote:

> On Tue, Aug 1, 2017 at 12:24 AM, Danilo Krummrich
> <danilokrummr...@dk-develop.de> wrote:
> 
> 
> When you add DT bindings you have to CC devicet...@vger.kernel.org
> 
I will do prospectively.

> > +#include 
> 
> Use only:
> 
> #include 
> 
Done in v6.

> > +#include 
> 
> Should not be needed.
> 
> 
Removed in v6.

> > +static int ps2_gpio_write(struct serio *serio, unsigned char val)
> > +{
> > +   struct ps2_gpio_data *drvdata = serio->port_data;
> > +
> > +   drvdata->mode = PS2_MODE_TX;
> > +   drvdata->tx_byte = val;
> > +   /* Make sure ISR running on other CPU notice changes. */
> > +   barrier();
> 
> This seems overengineered, is this really needed?
> 
> If we have races like this, the error is likely elsewhere, and should be
> fixed in the GPIO driver MMIO access or so.
> 
Yes, seems it can be removed. I didn't saw any explicit barriers in the GPIO
driver (I'm testing on bcm2835), but it seems MMIO operations on SMP archs
does contain barriers. Not sure if all do. If some do not this barrier might
be needed to ensure ISR on other CPU notice the correct mode and byte to send.

-- 
Danilo Krummrich <danilokrummr...@dk-develop.de>

Re: [PATCH] serio: PS2 gpio bit banging driver for the serio bus

[Danilo Krummrich]

Hi Linus,

thanks for reviewing. Commented the ones which still holds.

On Mon, 7 Aug 2017 11:03:53 +0200
Linus Walleij  wrote:

> On Tue, Aug 1, 2017 at 12:24 AM, Danilo Krummrich
>  wrote:
> 
> 
> When you add DT bindings you have to CC devicet...@vger.kernel.org
> 
I will do prospectively.

> > +#include 
> 
> Use only:
> 
> #include 
> 
Done in v6.

> > +#include 
> 
> Should not be needed.
> 
> 
Removed in v6.

> > +static int ps2_gpio_write(struct serio *serio, unsigned char val)
> > +{
> > +   struct ps2_gpio_data *drvdata = serio->port_data;
> > +
> > +   drvdata->mode = PS2_MODE_TX;
> > +   drvdata->tx_byte = val;
> > +   /* Make sure ISR running on other CPU notice changes. */
> > +   barrier();
> 
> This seems overengineered, is this really needed?
> 
> If we have races like this, the error is likely elsewhere, and should be
> fixed in the GPIO driver MMIO access or so.
> 
Yes, seems it can be removed. I didn't saw any explicit barriers in the GPIO
driver (I'm testing on bcm2835), but it seems MMIO operations on SMP archs
does contain barriers. Not sure if all do. If some do not this barrier might
be needed to ensure ISR on other CPU notice the correct mode and byte to send.

-- 
Danilo Krummrich 

[PATCH v6] serio: PS/2 gpio bit banging driver for serio bus

[Danilo Krummrich]

This driver provides PS/2 serio bus support by implementing bit banging
with the GPIO API. The GPIO pins, data and clock, can be configured with
a node in the device tree or by generic device properties (GDP).

Writing to a device is supported as well, though it is possible timings
can not be halt as they are tough and difficult to reach with bit banging.
Therefore it can be configured (also in DT and GDP) whether the serio
write function should be available for clients.

This driver is for development purposes and not recommended for productive
use. However, this driver can be useful e.g. when no USB port is available
or using old peripherals is desired as PS/2 controller chips getting rare.

This driver was tested on RPI1 and on Hikey960 and it worked well together
with the atkbd and psmouse driver.

Signed-off-by: Danilo Krummrich <danilokrummr...@dk-develop.de>
---
v2: Removed one verbose print statement, changed another one to dev_dbg.
v3: - fixed compiler warning on blackfin
- depends on GPIOLIB
- clarify documentation
v4: - fixed concurrent calls to ps2_gpio_write (serio->write)
- use gpiod API
- use generic device properties
- request irq separately, do not use gpiod_to_irq
- abort when gpio is connected via slow bus
- Fixed a bug where PS2_CMD_RESEND is always send after tx failed once.
  The makes the write functionallity work better, tough timing is still
  critical.
- disable irq initially until ps2_gpip_open (serio->open) is called
v5: Checked again why timings are that hard to reach while in tx mode and
discovered that there is an extra clock pulse between stop bit sent from
host and acknowledgement from device. By just skipping this clock pulse
tx works fine now, though it still happens sometimes that the timing can
not be reached of course.
v6: - fixed typos
- use of_match_ptr
---
 .../devicetree/bindings/serio/ps2-gpio.txt |  22 +
 Documentation/gpio/drivers-on-gpio.txt |   5 +
 drivers/input/serio/Kconfig|  11 +
 drivers/input/serio/Makefile   |   1 +
 drivers/input/serio/ps2-gpio.c | 455 +
 5 files changed, 494 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/serio/ps2-gpio.txt
 create mode 100644 drivers/input/serio/ps2-gpio.c

diff --git a/Documentation/devicetree/bindings/serio/ps2-gpio.txt 
b/Documentation/devicetree/bindings/serio/ps2-gpio.txt
new file mode 100644
index 000..099dd6d
--- /dev/null
+++ b/Documentation/devicetree/bindings/serio/ps2-gpio.txt
@@ -0,0 +1,22 @@
+Device-Tree bindings for ps/2 gpio driver
+
+Required properties:
+   - compatible = "ps2-gpio"
+   - gpios: data and clock gpio
+   - interrupts: Should trigger on the falling edge of the clock line.
+
+Optional properties:
+   - ps2-gpio,write-enable: Indicates whether write function is provided
+   to serio device. Possibly providing the write fn will not work, because
+   of the tough timing requirements.
+
+Example nodes:
+
+ps2@0 {
+   compatible = "ps2-gpio";
+   interrupt-parent = <>;
+   interrupts = <23 IRQ_TYPE_EDGE_FALLING>;
+   data-gpios = < 24 GPIO_ACTIVE_HIGH>;
+   clk-gpios = < 23 GPIO_ACTIVE_HIGH>;
+   ps2-gpio,write-enable;
+};
diff --git a/Documentation/gpio/drivers-on-gpio.txt 
b/Documentation/gpio/drivers-on-gpio.txt
index 3065132..9a78d38 100644
--- a/Documentation/gpio/drivers-on-gpio.txt
+++ b/Documentation/gpio/drivers-on-gpio.txt
@@ -84,6 +84,11 @@ hardware descriptions such as device tree or ACPI:
   NAND flash MTD subsystem and provides chip access and partition parsing like
   any other NAND driving hardware.
 
+- ps2-gpio: drivers/input/serio/ps2-gpio.c is used to drive a PS/2 (IBM) serio
+  bus, data and clock line, by bit banging two GPIO lines. It will appear as
+  any other serio bus to the system and makes it possible to connect drivers
+  for e.g. keyboards and other PS/2 protocol based devices.
+
 Apart from this there are special GPIO drivers in subsystems like MMC/SD to
 read card detect and write protect GPIO lines, and in the TTY serial subsystem
 to emulate MCTRL (modem control) signals CTS/RTS by using two GPIO lines. The
diff --git a/drivers/input/serio/Kconfig b/drivers/input/serio/Kconfig
index c3d05b4..21488c0 100644
--- a/drivers/input/serio/Kconfig
+++ b/drivers/input/serio/Kconfig
@@ -292,6 +292,17 @@ config SERIO_SUN4I_PS2
  To compile this driver as a module, choose M here: the
  module will be called sun4i-ps2.
 
+config SERIO_GPIO_PS2
+   tristate "GPIO PS/2 bit banging driver"
+   depends on GPIOLIB
+   help
+ Say Y here if you want PS/2 bit banging support via GPIO.
+
+ To compile this driver as a module, choose M here: the
+ module will be called ps2-gpio.
+
+ If you are unsure, say N.
+
 co

[PATCH v6] serio: PS/2 gpio bit banging driver for serio bus

[Danilo Krummrich]

This driver provides PS/2 serio bus support by implementing bit banging
with the GPIO API. The GPIO pins, data and clock, can be configured with
a node in the device tree or by generic device properties (GDP).

Writing to a device is supported as well, though it is possible timings
can not be halt as they are tough and difficult to reach with bit banging.
Therefore it can be configured (also in DT and GDP) whether the serio
write function should be available for clients.

This driver is for development purposes and not recommended for productive
use. However, this driver can be useful e.g. when no USB port is available
or using old peripherals is desired as PS/2 controller chips getting rare.

This driver was tested on RPI1 and on Hikey960 and it worked well together
with the atkbd and psmouse driver.

Signed-off-by: Danilo Krummrich 
---
v2: Removed one verbose print statement, changed another one to dev_dbg.
v3: - fixed compiler warning on blackfin
- depends on GPIOLIB
- clarify documentation
v4: - fixed concurrent calls to ps2_gpio_write (serio->write)
- use gpiod API
- use generic device properties
- request irq separately, do not use gpiod_to_irq
- abort when gpio is connected via slow bus
- Fixed a bug where PS2_CMD_RESEND is always send after tx failed once.
  The makes the write functionallity work better, tough timing is still
  critical.
- disable irq initially until ps2_gpip_open (serio->open) is called
v5: Checked again why timings are that hard to reach while in tx mode and
discovered that there is an extra clock pulse between stop bit sent from
host and acknowledgement from device. By just skipping this clock pulse
tx works fine now, though it still happens sometimes that the timing can
not be reached of course.
v6: - fixed typos
- use of_match_ptr
---
 .../devicetree/bindings/serio/ps2-gpio.txt |  22 +
 Documentation/gpio/drivers-on-gpio.txt |   5 +
 drivers/input/serio/Kconfig|  11 +
 drivers/input/serio/Makefile   |   1 +
 drivers/input/serio/ps2-gpio.c | 455 +
 5 files changed, 494 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/serio/ps2-gpio.txt
 create mode 100644 drivers/input/serio/ps2-gpio.c

diff --git a/Documentation/devicetree/bindings/serio/ps2-gpio.txt 
b/Documentation/devicetree/bindings/serio/ps2-gpio.txt
new file mode 100644
index 000..099dd6d
--- /dev/null
+++ b/Documentation/devicetree/bindings/serio/ps2-gpio.txt
@@ -0,0 +1,22 @@
+Device-Tree bindings for ps/2 gpio driver
+
+Required properties:
+   - compatible = "ps2-gpio"
+   - gpios: data and clock gpio
+   - interrupts: Should trigger on the falling edge of the clock line.
+
+Optional properties:
+   - ps2-gpio,write-enable: Indicates whether write function is provided
+   to serio device. Possibly providing the write fn will not work, because
+   of the tough timing requirements.
+
+Example nodes:
+
+ps2@0 {
+   compatible = "ps2-gpio";
+   interrupt-parent = <>;
+   interrupts = <23 IRQ_TYPE_EDGE_FALLING>;
+   data-gpios = < 24 GPIO_ACTIVE_HIGH>;
+   clk-gpios = < 23 GPIO_ACTIVE_HIGH>;
+   ps2-gpio,write-enable;
+};
diff --git a/Documentation/gpio/drivers-on-gpio.txt 
b/Documentation/gpio/drivers-on-gpio.txt
index 3065132..9a78d38 100644
--- a/Documentation/gpio/drivers-on-gpio.txt
+++ b/Documentation/gpio/drivers-on-gpio.txt
@@ -84,6 +84,11 @@ hardware descriptions such as device tree or ACPI:
   NAND flash MTD subsystem and provides chip access and partition parsing like
   any other NAND driving hardware.
 
+- ps2-gpio: drivers/input/serio/ps2-gpio.c is used to drive a PS/2 (IBM) serio
+  bus, data and clock line, by bit banging two GPIO lines. It will appear as
+  any other serio bus to the system and makes it possible to connect drivers
+  for e.g. keyboards and other PS/2 protocol based devices.
+
 Apart from this there are special GPIO drivers in subsystems like MMC/SD to
 read card detect and write protect GPIO lines, and in the TTY serial subsystem
 to emulate MCTRL (modem control) signals CTS/RTS by using two GPIO lines. The
diff --git a/drivers/input/serio/Kconfig b/drivers/input/serio/Kconfig
index c3d05b4..21488c0 100644
--- a/drivers/input/serio/Kconfig
+++ b/drivers/input/serio/Kconfig
@@ -292,6 +292,17 @@ config SERIO_SUN4I_PS2
  To compile this driver as a module, choose M here: the
  module will be called sun4i-ps2.
 
+config SERIO_GPIO_PS2
+   tristate "GPIO PS/2 bit banging driver"
+   depends on GPIOLIB
+   help
+ Say Y here if you want PS/2 bit banging support via GPIO.
+
+ To compile this driver as a module, choose M here: the
+ module will be called ps2-gpio.
+
+ If you are unsure, say N.
+
 config USERIO
tristate "User space s

[PATCH v5] serio: PS/2 gpio bit banging driver for serio bus

[Danilo Krummrich]

This driver provides PS/2 serio bus support by implementing bit banging
with the GPIO API. The GPIO pins, data and clock, can be configured with
a node in the device tree or by generic device properties (GDP).

Writing to a device is supported as well, though it is possible timings
can not be halt as they are tough and difficult to reach with bit banging.
Therefore it can be configured (also in DT and GDP) whether the serio
write function should be available for clients.

This driver is for development purposes and not recommended for productive
use. However, this driver can be useful e.g. when no USB port is available
or using old peripherals is desired as PS/2 controller chips getting rare.

This driver was tested on RPI1 and on Hikey960 and it worked well together
with the atkbd and psmouse driver.

Signed-off-by: Danilo Krummrich <danilokrummr...@dk-develop.de>
---
v2: Removed one verbose print statement, changed another one to dev_dbg.
v3: - fixed compiler warning on blackfin
- depends on GPIOLIB
- clarify documentation
v4: - fixed concurrent calls to ps2_gpio_write (serio->write)
- use gpiod API
- use generic device properties
- request irq separately, do not use gpiod_to_irq
- abort when gpio is connected via slow bus
- Fixed a bug where PS2_CMD_RESEND is always send after tx failed once.
  The makes the write functionallity work better, tough timing is still
  critical.
- disable irq initially until ps2_gpip_open (serio->open) is called
v5: Checked again why timings are that hard to reach while in tx mode and
discovered that there is an extra clock pulse between stop bit sent from
host and acknowledgement from device. By just skipping this clock pulse
tx works fine now, though it still happens sometimes that the timing can
not be reached of course.
---
 .../devicetree/bindings/serio/ps2-gpio.txt |  22 +
 Documentation/gpio/drivers-on-gpio.txt |   5 +
 drivers/input/serio/Kconfig|  11 +
 drivers/input/serio/Makefile   |   1 +
 drivers/input/serio/ps2-gpio.c | 456 +
 5 files changed, 495 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/serio/ps2-gpio.txt
 create mode 100644 drivers/input/serio/ps2-gpio.c

diff --git a/Documentation/devicetree/bindings/serio/ps2-gpio.txt 
b/Documentation/devicetree/bindings/serio/ps2-gpio.txt
new file mode 100644
index 000..19d42ca
--- /dev/null
+++ b/Documentation/devicetree/bindings/serio/ps2-gpio.txt
@@ -0,0 +1,22 @@
+Device-Tree bindings for ps2 gpio driver
+
+Required properties:
+   - compatible = "ps2-gpio"
+   - gpios: data and clock gpio
+   - interrupts: Should trigger on the falling edge of the clock line.
+
+Optional properties:
+   - ps2-gpio,write-enable: Indicates whether write function is provided
+   to serio device. Possibly providing the write fn will not work, because
+   of the tough timing requirements.
+
+Example nodes:
+
+ps2@0 {
+   compatible = "ps2-gpio";
+   interrupt-parent = <>;
+   interrupts = <23 IRQ_TYPE_EDGE_FALLING>;
+   data-gpios = < 24 GPIO_ACTIVE_HIGH>;
+   clk-gpios = < 23 GPIO_ACTIVE_HIGH>;
+   ps2-gpio,write-enable;
+};
diff --git a/Documentation/gpio/drivers-on-gpio.txt 
b/Documentation/gpio/drivers-on-gpio.txt
index 3065132..97c8716 100644
--- a/Documentation/gpio/drivers-on-gpio.txt
+++ b/Documentation/gpio/drivers-on-gpio.txt
@@ -84,6 +84,11 @@ hardware descriptions such as device tree or ACPI:
   NAND flash MTD subsystem and provides chip access and partition parsing like
   any other NAND driving hardware.
 
+- ps2-gpio: drivers/input/serio/ps2-gpio.c is used to drive an PS/2 (IBM) serio
+  bus, data and clock line, by bit banging two GPIO lines. It will appear as
+  any other serio bus to the system and makes it possible to connect drivers
+  for e.g. keyboards and other PS/2 protocol based devices.
+
 Apart from this there are special GPIO drivers in subsystems like MMC/SD to
 read card detect and write protect GPIO lines, and in the TTY serial subsystem
 to emulate MCTRL (modem control) signals CTS/RTS by using two GPIO lines. The
diff --git a/drivers/input/serio/Kconfig b/drivers/input/serio/Kconfig
index c3d05b4..292d6e2 100644
--- a/drivers/input/serio/Kconfig
+++ b/drivers/input/serio/Kconfig
@@ -292,6 +292,17 @@ config SERIO_SUN4I_PS2
  To compile this driver as a module, choose M here: the
  module will be called sun4i-ps2.
 
+config SERIO_GPIO_PS2
+   tristate "GPIO PS/2 bit banging driver"
+   depends on GPIOLIB
+   help
+ Say Y here if you want PS/2 bit banging support via GPIO.
+
+ To compile this driver as a module, choose M here: the
+ module will be called gpio-ps2.
+
+ If you are unsure, say N.
+
 config USERIO
tristate "User space s

[PATCH v5] serio: PS/2 gpio bit banging driver for serio bus

[Danilo Krummrich]

This driver provides PS/2 serio bus support by implementing bit banging
with the GPIO API. The GPIO pins, data and clock, can be configured with
a node in the device tree or by generic device properties (GDP).

Writing to a device is supported as well, though it is possible timings
can not be halt as they are tough and difficult to reach with bit banging.
Therefore it can be configured (also in DT and GDP) whether the serio
write function should be available for clients.

This driver is for development purposes and not recommended for productive
use. However, this driver can be useful e.g. when no USB port is available
or using old peripherals is desired as PS/2 controller chips getting rare.

This driver was tested on RPI1 and on Hikey960 and it worked well together
with the atkbd and psmouse driver.

Signed-off-by: Danilo Krummrich 
---
v2: Removed one verbose print statement, changed another one to dev_dbg.
v3: - fixed compiler warning on blackfin
- depends on GPIOLIB
- clarify documentation
v4: - fixed concurrent calls to ps2_gpio_write (serio->write)
- use gpiod API
- use generic device properties
- request irq separately, do not use gpiod_to_irq
- abort when gpio is connected via slow bus
- Fixed a bug where PS2_CMD_RESEND is always send after tx failed once.
  The makes the write functionallity work better, tough timing is still
  critical.
- disable irq initially until ps2_gpip_open (serio->open) is called
v5: Checked again why timings are that hard to reach while in tx mode and
discovered that there is an extra clock pulse between stop bit sent from
host and acknowledgement from device. By just skipping this clock pulse
tx works fine now, though it still happens sometimes that the timing can
not be reached of course.
---
 .../devicetree/bindings/serio/ps2-gpio.txt |  22 +
 Documentation/gpio/drivers-on-gpio.txt |   5 +
 drivers/input/serio/Kconfig|  11 +
 drivers/input/serio/Makefile   |   1 +
 drivers/input/serio/ps2-gpio.c | 456 +
 5 files changed, 495 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/serio/ps2-gpio.txt
 create mode 100644 drivers/input/serio/ps2-gpio.c

diff --git a/Documentation/devicetree/bindings/serio/ps2-gpio.txt 
b/Documentation/devicetree/bindings/serio/ps2-gpio.txt
new file mode 100644
index 000..19d42ca
--- /dev/null
+++ b/Documentation/devicetree/bindings/serio/ps2-gpio.txt
@@ -0,0 +1,22 @@
+Device-Tree bindings for ps2 gpio driver
+
+Required properties:
+   - compatible = "ps2-gpio"
+   - gpios: data and clock gpio
+   - interrupts: Should trigger on the falling edge of the clock line.
+
+Optional properties:
+   - ps2-gpio,write-enable: Indicates whether write function is provided
+   to serio device. Possibly providing the write fn will not work, because
+   of the tough timing requirements.
+
+Example nodes:
+
+ps2@0 {
+   compatible = "ps2-gpio";
+   interrupt-parent = <>;
+   interrupts = <23 IRQ_TYPE_EDGE_FALLING>;
+   data-gpios = < 24 GPIO_ACTIVE_HIGH>;
+   clk-gpios = < 23 GPIO_ACTIVE_HIGH>;
+   ps2-gpio,write-enable;
+};
diff --git a/Documentation/gpio/drivers-on-gpio.txt 
b/Documentation/gpio/drivers-on-gpio.txt
index 3065132..97c8716 100644
--- a/Documentation/gpio/drivers-on-gpio.txt
+++ b/Documentation/gpio/drivers-on-gpio.txt
@@ -84,6 +84,11 @@ hardware descriptions such as device tree or ACPI:
   NAND flash MTD subsystem and provides chip access and partition parsing like
   any other NAND driving hardware.
 
+- ps2-gpio: drivers/input/serio/ps2-gpio.c is used to drive an PS/2 (IBM) serio
+  bus, data and clock line, by bit banging two GPIO lines. It will appear as
+  any other serio bus to the system and makes it possible to connect drivers
+  for e.g. keyboards and other PS/2 protocol based devices.
+
 Apart from this there are special GPIO drivers in subsystems like MMC/SD to
 read card detect and write protect GPIO lines, and in the TTY serial subsystem
 to emulate MCTRL (modem control) signals CTS/RTS by using two GPIO lines. The
diff --git a/drivers/input/serio/Kconfig b/drivers/input/serio/Kconfig
index c3d05b4..292d6e2 100644
--- a/drivers/input/serio/Kconfig
+++ b/drivers/input/serio/Kconfig
@@ -292,6 +292,17 @@ config SERIO_SUN4I_PS2
  To compile this driver as a module, choose M here: the
  module will be called sun4i-ps2.
 
+config SERIO_GPIO_PS2
+   tristate "GPIO PS/2 bit banging driver"
+   depends on GPIOLIB
+   help
+ Say Y here if you want PS/2 bit banging support via GPIO.
+
+ To compile this driver as a module, choose M here: the
+ module will be called gpio-ps2.
+
+ If you are unsure, say N.
+
 config USERIO
tristate "User space serio port driver support"
  

[PATCH v4] serio: PS/2 gpio bit banging driver for serio bus

[Danilo Krummrich]

This driver provides PS/2 serio bus support by implementing bit banging
with the GPIO API. The GPIO pins, data and clock, can be configured with
a node in the device tree or by generic device properties (GDP).

Writing to a device is supported as well, though it is not recommended as
the timings to be halt given by libps2 are very tough and difficult to
reach with bit banging. Therefore it can be configured (also in DT and
GDP) whether the serio write function should be available for clients.

This driver is for development purposes and not for productive use.
However, this driver can be useful e.g. when no USB port is available or
using old peripherals is desired as PS/2 controller chips getting rare.

This driver was tested on RPI1 and on Hikey960 and it worked well together
with the atkbd driver.

Signed-off-by: Danilo Krummrich <danilokrummr...@dk-develop.de>
---
v2: Removed one verbose print statement, changed another one to dev_dbg.
v3: - fixed compiler warning on blackfin
- depends on GPIOLIB
- clarify documentation
v4: - fixed concurrent calls to ps2_gpio_write (serio->write)
- use gpiod API
- use generic device properties
- request irq separately, do not use gpiod_to_irq
- abort when gpio is connected via slow bus
- Fixed a bug where PS2_CMD_RESEND is always send after tx failed once.
  The makes the write functionallity work better, tough timing is still
  critical.
- disable irq initially until ps2_gpip_open (serio->open) is called
---
 .../devicetree/bindings/serio/ps2-gpio.txt |  22 +
 Documentation/gpio/drivers-on-gpio.txt |   5 +
 drivers/input/serio/Kconfig|  11 +
 drivers/input/serio/Makefile   |   1 +
 drivers/input/serio/ps2-gpio.c | 450 +
 5 files changed, 489 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/serio/ps2-gpio.txt
 create mode 100644 drivers/input/serio/ps2-gpio.c

diff --git a/Documentation/devicetree/bindings/serio/ps2-gpio.txt 
b/Documentation/devicetree/bindings/serio/ps2-gpio.txt
new file mode 100644
index 000..d825c8e
--- /dev/null
+++ b/Documentation/devicetree/bindings/serio/ps2-gpio.txt
@@ -0,0 +1,22 @@
+Device-Tree bindings for ps2 gpio driver
+
+Required properties:
+   - compatible = "ps2-gpio"
+   - gpios: data and clock gpio
+   - interrupts: Should trigger on the falling edge of the clock line.
+
+Optional properties:
+   - ps2-gpio,write-enable: Indicates whether write function is provided
+   to serio device. Probably providing the write fn will not work, because
+   of the tough timing libps2 requires.
+
+Example nodes:
+
+ps2@0 {
+   compatible = "ps2-gpio";
+   interrupt-parent = <>;
+   interrupts = <23 IRQ_TYPE_EDGE_FALLING>;
+   data-gpios = < 24 GPIO_ACTIVE_HIGH>;
+   clk-gpios = < 23 GPIO_ACTIVE_HIGH>;
+   ps2-gpio,write-enable;
+};
diff --git a/Documentation/gpio/drivers-on-gpio.txt 
b/Documentation/gpio/drivers-on-gpio.txt
index 3065132..97c8716 100644
--- a/Documentation/gpio/drivers-on-gpio.txt
+++ b/Documentation/gpio/drivers-on-gpio.txt
@@ -84,6 +84,11 @@ hardware descriptions such as device tree or ACPI:
   NAND flash MTD subsystem and provides chip access and partition parsing like
   any other NAND driving hardware.
 
+- ps2-gpio: drivers/input/serio/ps2-gpio.c is used to drive an PS/2 (IBM) serio
+  bus, data and clock line, by bit banging two GPIO lines. It will appear as
+  any other serio bus to the system and makes it possible to connect drivers
+  for e.g. keyboards and other PS/2 protocol based devices.
+
 Apart from this there are special GPIO drivers in subsystems like MMC/SD to
 read card detect and write protect GPIO lines, and in the TTY serial subsystem
 to emulate MCTRL (modem control) signals CTS/RTS by using two GPIO lines. The
diff --git a/drivers/input/serio/Kconfig b/drivers/input/serio/Kconfig
index c3d05b4..292d6e2 100644
--- a/drivers/input/serio/Kconfig
+++ b/drivers/input/serio/Kconfig
@@ -292,6 +292,17 @@ config SERIO_SUN4I_PS2
  To compile this driver as a module, choose M here: the
  module will be called sun4i-ps2.
 
+config SERIO_GPIO_PS2
+   tristate "GPIO PS/2 bit banging driver"
+   depends on GPIOLIB
+   help
+ Say Y here if you want PS/2 bit banging support via GPIO.
+
+ To compile this driver as a module, choose M here: the
+ module will be called gpio-ps2.
+
+ If you are unsure, say N.
+
 config USERIO
tristate "User space serio port driver support"
help
diff --git a/drivers/input/serio/Makefile b/drivers/input/serio/Makefile
index 2374ef9..767bd9b 100644
--- a/drivers/input/serio/Makefile
+++ b/drivers/input/serio/Makefile
@@ -30,4 +30,5 @@ obj-$(CONFIG_SERIO_APBPS2)+= apbps2.o
 obj-$(CONFIG_SERIO_OLPC_APSP)  += olpc_apsp.o
 obj-$

[PATCH v4] serio: PS/2 gpio bit banging driver for serio bus

[Danilo Krummrich]

This driver provides PS/2 serio bus support by implementing bit banging
with the GPIO API. The GPIO pins, data and clock, can be configured with
a node in the device tree or by generic device properties (GDP).

Writing to a device is supported as well, though it is not recommended as
the timings to be halt given by libps2 are very tough and difficult to
reach with bit banging. Therefore it can be configured (also in DT and
GDP) whether the serio write function should be available for clients.

This driver is for development purposes and not for productive use.
However, this driver can be useful e.g. when no USB port is available or
using old peripherals is desired as PS/2 controller chips getting rare.

This driver was tested on RPI1 and on Hikey960 and it worked well together
with the atkbd driver.

Signed-off-by: Danilo Krummrich 
---
v2: Removed one verbose print statement, changed another one to dev_dbg.
v3: - fixed compiler warning on blackfin
- depends on GPIOLIB
- clarify documentation
v4: - fixed concurrent calls to ps2_gpio_write (serio->write)
- use gpiod API
- use generic device properties
- request irq separately, do not use gpiod_to_irq
- abort when gpio is connected via slow bus
- Fixed a bug where PS2_CMD_RESEND is always send after tx failed once.
  The makes the write functionallity work better, tough timing is still
  critical.
- disable irq initially until ps2_gpip_open (serio->open) is called
---
 .../devicetree/bindings/serio/ps2-gpio.txt |  22 +
 Documentation/gpio/drivers-on-gpio.txt |   5 +
 drivers/input/serio/Kconfig|  11 +
 drivers/input/serio/Makefile   |   1 +
 drivers/input/serio/ps2-gpio.c | 450 +
 5 files changed, 489 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/serio/ps2-gpio.txt
 create mode 100644 drivers/input/serio/ps2-gpio.c

diff --git a/Documentation/devicetree/bindings/serio/ps2-gpio.txt 
b/Documentation/devicetree/bindings/serio/ps2-gpio.txt
new file mode 100644
index 000..d825c8e
--- /dev/null
+++ b/Documentation/devicetree/bindings/serio/ps2-gpio.txt
@@ -0,0 +1,22 @@
+Device-Tree bindings for ps2 gpio driver
+
+Required properties:
+   - compatible = "ps2-gpio"
+   - gpios: data and clock gpio
+   - interrupts: Should trigger on the falling edge of the clock line.
+
+Optional properties:
+   - ps2-gpio,write-enable: Indicates whether write function is provided
+   to serio device. Probably providing the write fn will not work, because
+   of the tough timing libps2 requires.
+
+Example nodes:
+
+ps2@0 {
+   compatible = "ps2-gpio";
+   interrupt-parent = <>;
+   interrupts = <23 IRQ_TYPE_EDGE_FALLING>;
+   data-gpios = < 24 GPIO_ACTIVE_HIGH>;
+   clk-gpios = < 23 GPIO_ACTIVE_HIGH>;
+   ps2-gpio,write-enable;
+};
diff --git a/Documentation/gpio/drivers-on-gpio.txt 
b/Documentation/gpio/drivers-on-gpio.txt
index 3065132..97c8716 100644
--- a/Documentation/gpio/drivers-on-gpio.txt
+++ b/Documentation/gpio/drivers-on-gpio.txt
@@ -84,6 +84,11 @@ hardware descriptions such as device tree or ACPI:
   NAND flash MTD subsystem and provides chip access and partition parsing like
   any other NAND driving hardware.
 
+- ps2-gpio: drivers/input/serio/ps2-gpio.c is used to drive an PS/2 (IBM) serio
+  bus, data and clock line, by bit banging two GPIO lines. It will appear as
+  any other serio bus to the system and makes it possible to connect drivers
+  for e.g. keyboards and other PS/2 protocol based devices.
+
 Apart from this there are special GPIO drivers in subsystems like MMC/SD to
 read card detect and write protect GPIO lines, and in the TTY serial subsystem
 to emulate MCTRL (modem control) signals CTS/RTS by using two GPIO lines. The
diff --git a/drivers/input/serio/Kconfig b/drivers/input/serio/Kconfig
index c3d05b4..292d6e2 100644
--- a/drivers/input/serio/Kconfig
+++ b/drivers/input/serio/Kconfig
@@ -292,6 +292,17 @@ config SERIO_SUN4I_PS2
  To compile this driver as a module, choose M here: the
  module will be called sun4i-ps2.
 
+config SERIO_GPIO_PS2
+   tristate "GPIO PS/2 bit banging driver"
+   depends on GPIOLIB
+   help
+ Say Y here if you want PS/2 bit banging support via GPIO.
+
+ To compile this driver as a module, choose M here: the
+ module will be called gpio-ps2.
+
+ If you are unsure, say N.
+
 config USERIO
tristate "User space serio port driver support"
help
diff --git a/drivers/input/serio/Makefile b/drivers/input/serio/Makefile
index 2374ef9..767bd9b 100644
--- a/drivers/input/serio/Makefile
+++ b/drivers/input/serio/Makefile
@@ -30,4 +30,5 @@ obj-$(CONFIG_SERIO_APBPS2)+= apbps2.o
 obj-$(CONFIG_SERIO_OLPC_APSP)  += olpc_apsp.o
 obj-$(CONFIG_HYPERV_KEYBOARD)  +=

[PATCH v3] serio: PS/2 gpio bit banging driver for the serio bus

[Danilo Krummrich]

This driver provides PS/2 serio bus support by implementing bit banging
with the GPIO API. The GPIO pins, data and clock, can be configured with
a node in the device tree or by static platform data.

Writing to a device is supported as well, though it is not recommended as
the timings to be halt given by libps2 are very tough and difficult to
reach with bit banging. Therefore it can be configured (also in DT and
pdata) whether the serio write function should be available for clients.

This driver is for development purposes and not for productive use.
However, this driver can be useful e.g. when no USB port is available or
using old peripherals is desired as PS/2 controller chips getting rare.

This driver was tested on a RPI1 and on Hikey960 and it worked well
together with the atkbd driver.

Signed-off-by: Danilo Krummrich <danilokrummr...@dk-develop.de>
---
v2: Removed one verbose print statement, changed another one to dev_dbg.
v3: - fixed compiler warning on blackfin
- depends on GPIOLIB
- clarify documentation
---
 .../devicetree/bindings/serio/ps2-gpio.txt |  20 +
 Documentation/gpio/drivers-on-gpio.txt |   5 +
 drivers/input/serio/Kconfig|  11 +
 drivers/input/serio/Makefile   |   1 +
 drivers/input/serio/ps2-gpio.c | 439 +
 include/linux/ps2-gpio.h   |  27 ++
 6 files changed, 503 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/serio/ps2-gpio.txt
 create mode 100644 drivers/input/serio/ps2-gpio.c
 create mode 100644 include/linux/ps2-gpio.h

diff --git a/Documentation/devicetree/bindings/serio/ps2-gpio.txt 
b/Documentation/devicetree/bindings/serio/ps2-gpio.txt
new file mode 100644
index 000..828a5b6
--- /dev/null
+++ b/Documentation/devicetree/bindings/serio/ps2-gpio.txt
@@ -0,0 +1,20 @@
+Device-Tree bindings for ps2 gpio driver
+
+Required properties:
+   - compatible = "ps2-gpio";
+   - gpios: data and clock gpio
+
+Optional properties:
+   - ps2-gpio,write-enable: Indicates whether write function is provided
+   to serio device. Most probably providing the write fn will not work,
+   because of the tough timing libps2 requires.
+
+Example nodes:
+
+ps2@0 {
+   compatible = "ps2-gpio";
+   gpios = < 24 0 /* data */
+ 23 0 /* clock */
+   >;
+   i2c-gpio,write-enable = <0>;
+};
diff --git a/Documentation/gpio/drivers-on-gpio.txt 
b/Documentation/gpio/drivers-on-gpio.txt
index 3065132..97c8716 100644
--- a/Documentation/gpio/drivers-on-gpio.txt
+++ b/Documentation/gpio/drivers-on-gpio.txt
@@ -84,6 +84,11 @@ hardware descriptions such as device tree or ACPI:
   NAND flash MTD subsystem and provides chip access and partition parsing like
   any other NAND driving hardware.
 
+- ps2-gpio: drivers/input/serio/ps2-gpio.c is used to drive an PS/2 (IBM) serio
+  bus, data and clock line, by bit banging two GPIO lines. It will appear as
+  any other serio bus to the system and makes it possible to connect drivers
+  for e.g. keyboards and other PS/2 protocol based devices.
+
 Apart from this there are special GPIO drivers in subsystems like MMC/SD to
 read card detect and write protect GPIO lines, and in the TTY serial subsystem
 to emulate MCTRL (modem control) signals CTS/RTS by using two GPIO lines. The
diff --git a/drivers/input/serio/Kconfig b/drivers/input/serio/Kconfig
index c3d05b4..292d6e2 100644
--- a/drivers/input/serio/Kconfig
+++ b/drivers/input/serio/Kconfig
@@ -292,6 +292,17 @@ config SERIO_SUN4I_PS2
  To compile this driver as a module, choose M here: the
  module will be called sun4i-ps2.
 
+config SERIO_GPIO_PS2
+   tristate "GPIO PS/2 bit banging driver"
+   depends on GPIOLIB
+   help
+ Say Y here if you want PS/2 bit banging support via GPIO.
+
+ To compile this driver as a module, choose M here: the
+ module will be called gpio-ps2.
+
+ If you are unsure, say N.
+
 config USERIO
tristate "User space serio port driver support"
help
diff --git a/drivers/input/serio/Makefile b/drivers/input/serio/Makefile
index 2374ef9..767bd9b 100644
--- a/drivers/input/serio/Makefile
+++ b/drivers/input/serio/Makefile
@@ -30,4 +30,5 @@ obj-$(CONFIG_SERIO_APBPS2)+= apbps2.o
 obj-$(CONFIG_SERIO_OLPC_APSP)  += olpc_apsp.o
 obj-$(CONFIG_HYPERV_KEYBOARD)  += hyperv-keyboard.o
 obj-$(CONFIG_SERIO_SUN4I_PS2)  += sun4i-ps2.o
+obj-$(CONFIG_SERIO_GPIO_PS2)   += ps2-gpio.o
 obj-$(CONFIG_USERIO)   += userio.o
diff --git a/drivers/input/serio/ps2-gpio.c b/drivers/input/serio/ps2-gpio.c
new file mode 100644
index 000..fc5368b
--- /dev/null
+++ b/drivers/input/serio/ps2-gpio.c
@@ -0,0 +1,439 @@
+/*
+ * GPIO based serio bus driver for bit banging the PS/2 protocol
+ *
+ * Author: Danilo Krummrich <danilokrummr...@dk-develop.de>
+ *
+ * This

[PATCH v3] serio: PS/2 gpio bit banging driver for the serio bus

[Danilo Krummrich]

This driver provides PS/2 serio bus support by implementing bit banging
with the GPIO API. The GPIO pins, data and clock, can be configured with
a node in the device tree or by static platform data.

Writing to a device is supported as well, though it is not recommended as
the timings to be halt given by libps2 are very tough and difficult to
reach with bit banging. Therefore it can be configured (also in DT and
pdata) whether the serio write function should be available for clients.

This driver is for development purposes and not for productive use.
However, this driver can be useful e.g. when no USB port is available or
using old peripherals is desired as PS/2 controller chips getting rare.

This driver was tested on a RPI1 and on Hikey960 and it worked well
together with the atkbd driver.

Signed-off-by: Danilo Krummrich 
---
v2: Removed one verbose print statement, changed another one to dev_dbg.
v3: - fixed compiler warning on blackfin
- depends on GPIOLIB
- clarify documentation
---
 .../devicetree/bindings/serio/ps2-gpio.txt |  20 +
 Documentation/gpio/drivers-on-gpio.txt |   5 +
 drivers/input/serio/Kconfig|  11 +
 drivers/input/serio/Makefile   |   1 +
 drivers/input/serio/ps2-gpio.c | 439 +
 include/linux/ps2-gpio.h   |  27 ++
 6 files changed, 503 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/serio/ps2-gpio.txt
 create mode 100644 drivers/input/serio/ps2-gpio.c
 create mode 100644 include/linux/ps2-gpio.h

diff --git a/Documentation/devicetree/bindings/serio/ps2-gpio.txt 
b/Documentation/devicetree/bindings/serio/ps2-gpio.txt
new file mode 100644
index 000..828a5b6
--- /dev/null
+++ b/Documentation/devicetree/bindings/serio/ps2-gpio.txt
@@ -0,0 +1,20 @@
+Device-Tree bindings for ps2 gpio driver
+
+Required properties:
+   - compatible = "ps2-gpio";
+   - gpios: data and clock gpio
+
+Optional properties:
+   - ps2-gpio,write-enable: Indicates whether write function is provided
+   to serio device. Most probably providing the write fn will not work,
+   because of the tough timing libps2 requires.
+
+Example nodes:
+
+ps2@0 {
+   compatible = "ps2-gpio";
+   gpios = < 24 0 /* data */
+ 23 0 /* clock */
+   >;
+   i2c-gpio,write-enable = <0>;
+};
diff --git a/Documentation/gpio/drivers-on-gpio.txt 
b/Documentation/gpio/drivers-on-gpio.txt
index 3065132..97c8716 100644
--- a/Documentation/gpio/drivers-on-gpio.txt
+++ b/Documentation/gpio/drivers-on-gpio.txt
@@ -84,6 +84,11 @@ hardware descriptions such as device tree or ACPI:
   NAND flash MTD subsystem and provides chip access and partition parsing like
   any other NAND driving hardware.
 
+- ps2-gpio: drivers/input/serio/ps2-gpio.c is used to drive an PS/2 (IBM) serio
+  bus, data and clock line, by bit banging two GPIO lines. It will appear as
+  any other serio bus to the system and makes it possible to connect drivers
+  for e.g. keyboards and other PS/2 protocol based devices.
+
 Apart from this there are special GPIO drivers in subsystems like MMC/SD to
 read card detect and write protect GPIO lines, and in the TTY serial subsystem
 to emulate MCTRL (modem control) signals CTS/RTS by using two GPIO lines. The
diff --git a/drivers/input/serio/Kconfig b/drivers/input/serio/Kconfig
index c3d05b4..292d6e2 100644
--- a/drivers/input/serio/Kconfig
+++ b/drivers/input/serio/Kconfig
@@ -292,6 +292,17 @@ config SERIO_SUN4I_PS2
  To compile this driver as a module, choose M here: the
  module will be called sun4i-ps2.
 
+config SERIO_GPIO_PS2
+   tristate "GPIO PS/2 bit banging driver"
+   depends on GPIOLIB
+   help
+ Say Y here if you want PS/2 bit banging support via GPIO.
+
+ To compile this driver as a module, choose M here: the
+ module will be called gpio-ps2.
+
+ If you are unsure, say N.
+
 config USERIO
tristate "User space serio port driver support"
help
diff --git a/drivers/input/serio/Makefile b/drivers/input/serio/Makefile
index 2374ef9..767bd9b 100644
--- a/drivers/input/serio/Makefile
+++ b/drivers/input/serio/Makefile
@@ -30,4 +30,5 @@ obj-$(CONFIG_SERIO_APBPS2)+= apbps2.o
 obj-$(CONFIG_SERIO_OLPC_APSP)  += olpc_apsp.o
 obj-$(CONFIG_HYPERV_KEYBOARD)  += hyperv-keyboard.o
 obj-$(CONFIG_SERIO_SUN4I_PS2)  += sun4i-ps2.o
+obj-$(CONFIG_SERIO_GPIO_PS2)   += ps2-gpio.o
 obj-$(CONFIG_USERIO)   += userio.o
diff --git a/drivers/input/serio/ps2-gpio.c b/drivers/input/serio/ps2-gpio.c
new file mode 100644
index 000..fc5368b
--- /dev/null
+++ b/drivers/input/serio/ps2-gpio.c
@@ -0,0 +1,439 @@
+/*
+ * GPIO based serio bus driver for bit banging the PS/2 protocol
+ *
+ * Author: Danilo Krummrich 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under t

[PATCH v2] serio: PS2 gpio bit banging driver for the serio bus

[Danilo Krummrich]

This driver provides PS2 serio bus support by implementing bit banging
with the GPIO API. The GPIO pins, data and clock, can be configured with
a node in the device tree or by static platform data.

Writing to a device is supported as well, though it is not recommended as
the timings to be halt given by libps2 are very tough and difficult to
reach with bit banging. Therefore it can be configured (also in DT and
pdata) whether the serio write function should be available for clients.

This driver is for development purposes and not for productive use.
However, this driver can be useful e.g. when no USB port is available or
using old peripherals is desired as PS2 controllers getting rare.

This driver was tested on a RPI1 and on Hikey960 and it worked well
together with the atkbd driver.

Signed-off-by: Danilo Krummrich <danilokrummr...@dk-develop.de>
---
v2: Removed one verbose print statement, changed another one to dev_dbg.
---
 .../devicetree/bindings/serio/ps2-gpio.txt |  20 +
 Documentation/gpio/drivers-on-gpio.txt |   5 +
 drivers/input/serio/Kconfig|  10 +
 drivers/input/serio/Makefile   |   1 +
 drivers/input/serio/ps2-gpio.c | 438 +
 include/linux/ps2-gpio.h   |  27 ++
 6 files changed, 501 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/serio/ps2-gpio.txt
 create mode 100644 drivers/input/serio/ps2-gpio.c
 create mode 100644 include/linux/ps2-gpio.h

diff --git a/Documentation/devicetree/bindings/serio/ps2-gpio.txt 
b/Documentation/devicetree/bindings/serio/ps2-gpio.txt
new file mode 100644
index 000..828a5b6
--- /dev/null
+++ b/Documentation/devicetree/bindings/serio/ps2-gpio.txt
@@ -0,0 +1,20 @@
+Device-Tree bindings for ps2 gpio driver
+
+Required properties:
+   - compatible = "ps2-gpio";
+   - gpios: data and clock gpio
+
+Optional properties:
+   - ps2-gpio,write-enable: Indicates whether write function is provided
+   to serio device. Most probably providing the write fn will not work,
+   because of the tough timing libps2 requires.
+
+Example nodes:
+
+ps2@0 {
+   compatible = "ps2-gpio";
+   gpios = < 24 0 /* data */
+ 23 0 /* clock */
+   >;
+   i2c-gpio,write-enable = <0>;
+};
diff --git a/Documentation/gpio/drivers-on-gpio.txt 
b/Documentation/gpio/drivers-on-gpio.txt
index 3065132..b106e72 100644
--- a/Documentation/gpio/drivers-on-gpio.txt
+++ b/Documentation/gpio/drivers-on-gpio.txt
@@ -84,6 +84,11 @@ hardware descriptions such as device tree or ACPI:
   NAND flash MTD subsystem and provides chip access and partition parsing like
   any other NAND driving hardware.
 
+- ps2-gpio: drivers/input/serio/ps2-gpio.c is used to drive an PS2 serio bus,
+  data and clock line, by bit banging two GPIO lines. It will appear as any
+  other serio bus to the system and makes it possible to connect drivers for
+  e.g. keyboards and other PS2 protocol based devices.
+
 Apart from this there are special GPIO drivers in subsystems like MMC/SD to
 read card detect and write protect GPIO lines, and in the TTY serial subsystem
 to emulate MCTRL (modem control) signals CTS/RTS by using two GPIO lines. The
diff --git a/drivers/input/serio/Kconfig b/drivers/input/serio/Kconfig
index c3d05b4..abca0e0 100644
--- a/drivers/input/serio/Kconfig
+++ b/drivers/input/serio/Kconfig
@@ -292,6 +292,16 @@ config SERIO_SUN4I_PS2
  To compile this driver as a module, choose M here: the
  module will be called sun4i-ps2.
 
+config SERIO_GPIO_PS2
+   tristate "GPIO PS/2 bit banging driver"
+   help
+ Say Y here if you want PS/2 bit banging support via GPIO.
+
+ To compile this driver as a module, choose M here: the
+ module will be called gpio-ps2.
+
+ If you are unsure, say N.
+
 config USERIO
tristate "User space serio port driver support"
help
diff --git a/drivers/input/serio/Makefile b/drivers/input/serio/Makefile
index 2374ef9..767bd9b 100644
--- a/drivers/input/serio/Makefile
+++ b/drivers/input/serio/Makefile
@@ -30,4 +30,5 @@ obj-$(CONFIG_SERIO_APBPS2)+= apbps2.o
 obj-$(CONFIG_SERIO_OLPC_APSP)  += olpc_apsp.o
 obj-$(CONFIG_HYPERV_KEYBOARD)  += hyperv-keyboard.o
 obj-$(CONFIG_SERIO_SUN4I_PS2)  += sun4i-ps2.o
+obj-$(CONFIG_SERIO_GPIO_PS2)   += ps2-gpio.o
 obj-$(CONFIG_USERIO)   += userio.o
diff --git a/drivers/input/serio/ps2-gpio.c b/drivers/input/serio/ps2-gpio.c
new file mode 100644
index 000..cc9287e
--- /dev/null
+++ b/drivers/input/serio/ps2-gpio.c
@@ -0,0 +1,438 @@
+/*
+ * GPIO based serio bus driver for bit banging the PS2 protocol
+ *
+ * Author: Danilo Krummrich <danilokrummr...@dk-develop.de>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as

[PATCH v2] serio: PS2 gpio bit banging driver for the serio bus

[Danilo Krummrich]

This driver provides PS2 serio bus support by implementing bit banging
with the GPIO API. The GPIO pins, data and clock, can be configured with
a node in the device tree or by static platform data.

Writing to a device is supported as well, though it is not recommended as
the timings to be halt given by libps2 are very tough and difficult to
reach with bit banging. Therefore it can be configured (also in DT and
pdata) whether the serio write function should be available for clients.

This driver is for development purposes and not for productive use.
However, this driver can be useful e.g. when no USB port is available or
using old peripherals is desired as PS2 controllers getting rare.

This driver was tested on a RPI1 and on Hikey960 and it worked well
together with the atkbd driver.

Signed-off-by: Danilo Krummrich 
---
v2: Removed one verbose print statement, changed another one to dev_dbg.
---
 .../devicetree/bindings/serio/ps2-gpio.txt |  20 +
 Documentation/gpio/drivers-on-gpio.txt |   5 +
 drivers/input/serio/Kconfig|  10 +
 drivers/input/serio/Makefile   |   1 +
 drivers/input/serio/ps2-gpio.c | 438 +
 include/linux/ps2-gpio.h   |  27 ++
 6 files changed, 501 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/serio/ps2-gpio.txt
 create mode 100644 drivers/input/serio/ps2-gpio.c
 create mode 100644 include/linux/ps2-gpio.h

diff --git a/Documentation/devicetree/bindings/serio/ps2-gpio.txt 
b/Documentation/devicetree/bindings/serio/ps2-gpio.txt
new file mode 100644
index 000..828a5b6
--- /dev/null
+++ b/Documentation/devicetree/bindings/serio/ps2-gpio.txt
@@ -0,0 +1,20 @@
+Device-Tree bindings for ps2 gpio driver
+
+Required properties:
+   - compatible = "ps2-gpio";
+   - gpios: data and clock gpio
+
+Optional properties:
+   - ps2-gpio,write-enable: Indicates whether write function is provided
+   to serio device. Most probably providing the write fn will not work,
+   because of the tough timing libps2 requires.
+
+Example nodes:
+
+ps2@0 {
+   compatible = "ps2-gpio";
+   gpios = < 24 0 /* data */
+ 23 0 /* clock */
+   >;
+   i2c-gpio,write-enable = <0>;
+};
diff --git a/Documentation/gpio/drivers-on-gpio.txt 
b/Documentation/gpio/drivers-on-gpio.txt
index 3065132..b106e72 100644
--- a/Documentation/gpio/drivers-on-gpio.txt
+++ b/Documentation/gpio/drivers-on-gpio.txt
@@ -84,6 +84,11 @@ hardware descriptions such as device tree or ACPI:
   NAND flash MTD subsystem and provides chip access and partition parsing like
   any other NAND driving hardware.
 
+- ps2-gpio: drivers/input/serio/ps2-gpio.c is used to drive an PS2 serio bus,
+  data and clock line, by bit banging two GPIO lines. It will appear as any
+  other serio bus to the system and makes it possible to connect drivers for
+  e.g. keyboards and other PS2 protocol based devices.
+
 Apart from this there are special GPIO drivers in subsystems like MMC/SD to
 read card detect and write protect GPIO lines, and in the TTY serial subsystem
 to emulate MCTRL (modem control) signals CTS/RTS by using two GPIO lines. The
diff --git a/drivers/input/serio/Kconfig b/drivers/input/serio/Kconfig
index c3d05b4..abca0e0 100644
--- a/drivers/input/serio/Kconfig
+++ b/drivers/input/serio/Kconfig
@@ -292,6 +292,16 @@ config SERIO_SUN4I_PS2
  To compile this driver as a module, choose M here: the
  module will be called sun4i-ps2.
 
+config SERIO_GPIO_PS2
+   tristate "GPIO PS/2 bit banging driver"
+   help
+ Say Y here if you want PS/2 bit banging support via GPIO.
+
+ To compile this driver as a module, choose M here: the
+ module will be called gpio-ps2.
+
+ If you are unsure, say N.
+
 config USERIO
tristate "User space serio port driver support"
help
diff --git a/drivers/input/serio/Makefile b/drivers/input/serio/Makefile
index 2374ef9..767bd9b 100644
--- a/drivers/input/serio/Makefile
+++ b/drivers/input/serio/Makefile
@@ -30,4 +30,5 @@ obj-$(CONFIG_SERIO_APBPS2)+= apbps2.o
 obj-$(CONFIG_SERIO_OLPC_APSP)  += olpc_apsp.o
 obj-$(CONFIG_HYPERV_KEYBOARD)  += hyperv-keyboard.o
 obj-$(CONFIG_SERIO_SUN4I_PS2)  += sun4i-ps2.o
+obj-$(CONFIG_SERIO_GPIO_PS2)   += ps2-gpio.o
 obj-$(CONFIG_USERIO)   += userio.o
diff --git a/drivers/input/serio/ps2-gpio.c b/drivers/input/serio/ps2-gpio.c
new file mode 100644
index 000..cc9287e
--- /dev/null
+++ b/drivers/input/serio/ps2-gpio.c
@@ -0,0 +1,438 @@
+/*
+ * GPIO based serio bus driver for bit banging the PS2 protocol
+ *
+ * Author: Danilo Krummrich 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include 
+#include 
+

[PATCH] serio: PS2 gpio bit banging driver for the serio bus

[Danilo Krummrich]

This driver provides PS2 serio bus support by implementing bit banging
with the GPIO API. The GPIO pins, data and clock, can be configured with
a node in the device tree or by static platform data.

Writing to a device is supported as well, though it is not recommended as
the timings to be halt given by libps2 are very tough and difficult to
reach with bit banging. Therefore it can be configured (also in DT and
pdata) whether the serio write function should be available for clients.

This driver is for development purposes and not for productive use.
However, this driver can be useful e.g. when no USB port is available or
using old peripherals is desired as PS2 controllers getting rare.

This driver was tested on a RPI1 and on Hikey960 and it worked well
together with the atkbd driver.

Signed-off-by: Danilo Krummrich <danilokrummr...@dk-develop.de>
---
 .../devicetree/bindings/serio/ps2-gpio.txt |  20 +
 Documentation/gpio/drivers-on-gpio.txt |   5 +
 drivers/input/serio/Kconfig|  10 +
 drivers/input/serio/Makefile   |   1 +
 drivers/input/serio/ps2-gpio.c | 439 +
 include/linux/ps2-gpio.h   |  27 ++
 6 files changed, 502 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/serio/ps2-gpio.txt
 create mode 100644 drivers/input/serio/ps2-gpio.c
 create mode 100644 include/linux/ps2-gpio.h

diff --git a/Documentation/devicetree/bindings/serio/ps2-gpio.txt 
b/Documentation/devicetree/bindings/serio/ps2-gpio.txt
new file mode 100644
index 000..828a5b6
--- /dev/null
+++ b/Documentation/devicetree/bindings/serio/ps2-gpio.txt
@@ -0,0 +1,20 @@
+Device-Tree bindings for ps2 gpio driver
+
+Required properties:
+   - compatible = "ps2-gpio";
+   - gpios: data and clock gpio
+
+Optional properties:
+   - ps2-gpio,write-enable: Indicates whether write function is provided
+   to serio device. Most probably providing the write fn will not work,
+   because of the tough timing libps2 requires.
+
+Example nodes:
+
+ps2@0 {
+   compatible = "ps2-gpio";
+   gpios = < 24 0 /* data */
+ 23 0 /* clock */
+   >;
+   i2c-gpio,write-enable = <0>;
+};
diff --git a/Documentation/gpio/drivers-on-gpio.txt 
b/Documentation/gpio/drivers-on-gpio.txt
index 3065132..b106e72 100644
--- a/Documentation/gpio/drivers-on-gpio.txt
+++ b/Documentation/gpio/drivers-on-gpio.txt
@@ -84,6 +84,11 @@ hardware descriptions such as device tree or ACPI:
   NAND flash MTD subsystem and provides chip access and partition parsing like
   any other NAND driving hardware.
 
+- ps2-gpio: drivers/input/serio/ps2-gpio.c is used to drive an PS2 serio bus,
+  data and clock line, by bit banging two GPIO lines. It will appear as any
+  other serio bus to the system and makes it possible to connect drivers for
+  e.g. keyboards and other PS2 protocol based devices.
+
 Apart from this there are special GPIO drivers in subsystems like MMC/SD to
 read card detect and write protect GPIO lines, and in the TTY serial subsystem
 to emulate MCTRL (modem control) signals CTS/RTS by using two GPIO lines. The
diff --git a/drivers/input/serio/Kconfig b/drivers/input/serio/Kconfig
index c3d05b4..abca0e0 100644
--- a/drivers/input/serio/Kconfig
+++ b/drivers/input/serio/Kconfig
@@ -292,6 +292,16 @@ config SERIO_SUN4I_PS2
  To compile this driver as a module, choose M here: the
  module will be called sun4i-ps2.
 
+config SERIO_GPIO_PS2
+   tristate "GPIO PS/2 bit banging driver"
+   help
+ Say Y here if you want PS/2 bit banging support via GPIO.
+
+ To compile this driver as a module, choose M here: the
+ module will be called gpio-ps2.
+
+ If you are unsure, say N.
+
 config USERIO
tristate "User space serio port driver support"
help
diff --git a/drivers/input/serio/Makefile b/drivers/input/serio/Makefile
index 2374ef9..767bd9b 100644
--- a/drivers/input/serio/Makefile
+++ b/drivers/input/serio/Makefile
@@ -30,4 +30,5 @@ obj-$(CONFIG_SERIO_APBPS2)+= apbps2.o
 obj-$(CONFIG_SERIO_OLPC_APSP)  += olpc_apsp.o
 obj-$(CONFIG_HYPERV_KEYBOARD)  += hyperv-keyboard.o
 obj-$(CONFIG_SERIO_SUN4I_PS2)  += sun4i-ps2.o
+obj-$(CONFIG_SERIO_GPIO_PS2)   += ps2-gpio.o
 obj-$(CONFIG_USERIO)   += userio.o
diff --git a/drivers/input/serio/ps2-gpio.c b/drivers/input/serio/ps2-gpio.c
new file mode 100644
index 000..5b9f84f
--- /dev/null
+++ b/drivers/input/serio/ps2-gpio.c
@@ -0,0 +1,439 @@
+/*
+ * GPIO based serio bus driver for bit banging the PS2 protocol
+ *
+ * Author: Danilo Krummrich <danilokrummr...@dk-develop.de>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include 
+#include 

[PATCH] serio: PS2 gpio bit banging driver for the serio bus

[Danilo Krummrich]

This driver provides PS2 serio bus support by implementing bit banging
with the GPIO API. The GPIO pins, data and clock, can be configured with
a node in the device tree or by static platform data.

Writing to a device is supported as well, though it is not recommended as
the timings to be halt given by libps2 are very tough and difficult to
reach with bit banging. Therefore it can be configured (also in DT and
pdata) whether the serio write function should be available for clients.

This driver is for development purposes and not for productive use.
However, this driver can be useful e.g. when no USB port is available or
using old peripherals is desired as PS2 controllers getting rare.

This driver was tested on a RPI1 and on Hikey960 and it worked well
together with the atkbd driver.

Signed-off-by: Danilo Krummrich 
---
 .../devicetree/bindings/serio/ps2-gpio.txt |  20 +
 Documentation/gpio/drivers-on-gpio.txt |   5 +
 drivers/input/serio/Kconfig|  10 +
 drivers/input/serio/Makefile   |   1 +
 drivers/input/serio/ps2-gpio.c | 439 +
 include/linux/ps2-gpio.h   |  27 ++
 6 files changed, 502 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/serio/ps2-gpio.txt
 create mode 100644 drivers/input/serio/ps2-gpio.c
 create mode 100644 include/linux/ps2-gpio.h

diff --git a/Documentation/devicetree/bindings/serio/ps2-gpio.txt 
b/Documentation/devicetree/bindings/serio/ps2-gpio.txt
new file mode 100644
index 000..828a5b6
--- /dev/null
+++ b/Documentation/devicetree/bindings/serio/ps2-gpio.txt
@@ -0,0 +1,20 @@
+Device-Tree bindings for ps2 gpio driver
+
+Required properties:
+   - compatible = "ps2-gpio";
+   - gpios: data and clock gpio
+
+Optional properties:
+   - ps2-gpio,write-enable: Indicates whether write function is provided
+   to serio device. Most probably providing the write fn will not work,
+   because of the tough timing libps2 requires.
+
+Example nodes:
+
+ps2@0 {
+   compatible = "ps2-gpio";
+   gpios = < 24 0 /* data */
+ 23 0 /* clock */
+   >;
+   i2c-gpio,write-enable = <0>;
+};
diff --git a/Documentation/gpio/drivers-on-gpio.txt 
b/Documentation/gpio/drivers-on-gpio.txt
index 3065132..b106e72 100644
--- a/Documentation/gpio/drivers-on-gpio.txt
+++ b/Documentation/gpio/drivers-on-gpio.txt
@@ -84,6 +84,11 @@ hardware descriptions such as device tree or ACPI:
   NAND flash MTD subsystem and provides chip access and partition parsing like
   any other NAND driving hardware.
 
+- ps2-gpio: drivers/input/serio/ps2-gpio.c is used to drive an PS2 serio bus,
+  data and clock line, by bit banging two GPIO lines. It will appear as any
+  other serio bus to the system and makes it possible to connect drivers for
+  e.g. keyboards and other PS2 protocol based devices.
+
 Apart from this there are special GPIO drivers in subsystems like MMC/SD to
 read card detect and write protect GPIO lines, and in the TTY serial subsystem
 to emulate MCTRL (modem control) signals CTS/RTS by using two GPIO lines. The
diff --git a/drivers/input/serio/Kconfig b/drivers/input/serio/Kconfig
index c3d05b4..abca0e0 100644
--- a/drivers/input/serio/Kconfig
+++ b/drivers/input/serio/Kconfig
@@ -292,6 +292,16 @@ config SERIO_SUN4I_PS2
  To compile this driver as a module, choose M here: the
  module will be called sun4i-ps2.
 
+config SERIO_GPIO_PS2
+   tristate "GPIO PS/2 bit banging driver"
+   help
+ Say Y here if you want PS/2 bit banging support via GPIO.
+
+ To compile this driver as a module, choose M here: the
+ module will be called gpio-ps2.
+
+ If you are unsure, say N.
+
 config USERIO
tristate "User space serio port driver support"
help
diff --git a/drivers/input/serio/Makefile b/drivers/input/serio/Makefile
index 2374ef9..767bd9b 100644
--- a/drivers/input/serio/Makefile
+++ b/drivers/input/serio/Makefile
@@ -30,4 +30,5 @@ obj-$(CONFIG_SERIO_APBPS2)+= apbps2.o
 obj-$(CONFIG_SERIO_OLPC_APSP)  += olpc_apsp.o
 obj-$(CONFIG_HYPERV_KEYBOARD)  += hyperv-keyboard.o
 obj-$(CONFIG_SERIO_SUN4I_PS2)  += sun4i-ps2.o
+obj-$(CONFIG_SERIO_GPIO_PS2)   += ps2-gpio.o
 obj-$(CONFIG_USERIO)   += userio.o
diff --git a/drivers/input/serio/ps2-gpio.c b/drivers/input/serio/ps2-gpio.c
new file mode 100644
index 000..5b9f84f
--- /dev/null
+++ b/drivers/input/serio/ps2-gpio.c
@@ -0,0 +1,439 @@
+/*
+ * GPIO based serio bus driver for bit banging the PS2 protocol
+ *
+ * Author: Danilo Krummrich 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#i