Re: [PATCH net-next v2 0/2] of: mdio: Fall back to mdiobus_register() with NULL device_node
On Wed, May 16, 2018 at 10:54:12AM +0200, Geert Uytterhoeven wrote: > Hi Florian, > > Thanks for your series! > I like the effect on simplifying drivers. > > On Wed, May 16, 2018 at 1:56 AM, Florian Fainelli> wrote: > > This patch series updates of_mdiobus_register() such that when the > > device_node > > argument is NULL, it calls mdiobus_register() directly. This is consistent > > with > > the behavior of of_mdiobus_register() when CONFIG_OF=n. > > IMHO the CONFIG_OF=n behavior of of_mdiobus_register() (which I wasn't > aware of) is inconsistent with the behavior of other of_*() functions, > which are just empty stubs. > > So I'm wondering if you should do it the other way around, and let > mdiobus_register() call of_mdiobus_register() if dev->of_node exists? Hi Geert dev->of_node is often not the correct OF node. The mdio properties are often embedded inside a MAC driver, and use an 'mdio' container node. This container node is needed, not the device node. > I haven't looked at the ACPI handling, but perhaps this can be moved > inside mdiobus_register() as well? The ACPI binding for MDIO and PHYs has not been defined yet. Andrew -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 3/3] dt-bindings: Document the DT bindings for lan78xx
On Thu, Apr 19, 2018 at 03:32:05PM +0100, Phil Elwell wrote: > The Microchip LAN78XX family of devices are Ethernet controllers with > a USB interface. Despite being discoverable devices it can be useful to > be able to configure them from Device Tree, particularly in low-cost > applications without an EEPROM or programmed OTP. > > Document the supported properties in a bindings file. > > Signed-off-by: Phil Elwell <p...@raspberrypi.org> Reviewed-by: Andrew Lunn <and...@lunn.ch> Andrew -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 2/3] lan78xx: Read LED states from Device Tree
> @@ -2077,6 +2085,28 @@ static int lan78xx_phy_init(struct lan78xx_net *dev) > mii_adv = (u32)mii_advertise_flowctrl(dev->fc_request_control); > phydev->advertising |= mii_adv_to_ethtool_adv_t(mii_adv); > > + if (phydev->mdio.dev.of_node) { > + u32 reg; > + int len; > + > + len = of_property_count_elems_of_size(phydev->mdio.dev.of_node, > + "microchip,led-modes", > + sizeof(u32)); > + if (len >= 0) { > + /* Ensure the appropriate LEDs are enabled */ > + lan78xx_read_reg(dev, HW_CFG, ); > + reg &= ~(HW_CFG_LED0_EN_ | > + HW_CFG_LED1_EN_ | > + HW_CFG_LED2_EN_ | > + HW_CFG_LED3_EN_); > + reg |= (len > 0) * HW_CFG_LED0_EN_ | > + (len > 1) * HW_CFG_LED1_EN_ | > + (len > 2) * HW_CFG_LED2_EN_ | > + (len > 3) * HW_CFG_LED3_EN_; > + lan78xx_write_reg(dev, HW_CFG, reg); > + } > + } > + Humm. Not nice. But i cannot think of a cleaner way of doing this. Reviewed-by: Andrew Lunn <and...@lunn.ch> Andrew -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/3] lan78xx: Read LED states from Device Tree
On Wed, Apr 18, 2018 at 04:45:22PM +0100, Phil Elwell wrote: > Add support for DT property "microchip,led-modes", a vector of zero > to four cells (u32s) in the range 0-15, each of which sets the mode > for one of the LEDs. Some possible values are: > > 0=link/activity 1=link1000/activity > 2=link100/activity 3=link10/activity > 4=link100/1000/activity 5=link10/1000/activity > 6=link10/100/activity14=off15=on > > These values are given symbolic constants in a dt-bindings header. > > Also use the presence of the DT property to indicate that the > LEDs should be enabled - necessary in the event that no valid OTP > or EEPROM is available. Hi Phil As i said last week, these are PHY properties, so should be in the PHY node in device tree. It should be the PHY driver which parses these properties and configures the LEDs, not the MAC. Andrew -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] lan78xx: Read LED modes from Device Tree
> @@ -2097,6 +2098,25 @@ static int lan78xx_phy_init(struct lan78xx_net *dev) > (void)lan78xx_set_eee(dev->net, ); > } > > + if (!of_property_read_u32_array(dev->udev->dev.of_node, > + "microchip,led-modes", > + led_modes, ARRAY_SIZE(led_modes))) { > + u32 reg; > + int i; > + > + reg = phy_read(phydev, 0x1d); > + for (i = 0; i < ARRAY_SIZE(led_modes); i++) { > + reg &= ~(0xf << (i * 4)); > + reg |= (led_modes[i] & 0xf) << (i * 4); > + } > + (void)phy_write(phydev, 0x1d, reg); Poking PHY registers directly from the MAC driver is not always a good idea. This MAC driver does that in a few places :-( What do we know about the PHY? It is built into the device or is it external? If it is external, how do you know the LED register is at 0x1d? The safest place to do this is in the PHY driver, and place these OF properties into the PHY node. Andrew -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] dt-bindings: Document the DT bindings for lan78xx
On Thu, Apr 12, 2018 at 02:55:36PM +0100, Phil Elwell wrote: > The Microchip LAN78XX family of devices are Ethernet controllers with > a USB interface. Despite being discoverable devices it can be useful to > be able to configure them from Device Tree, particularly in low-cost > applications without an EEPROM or programmed OTP. It would be good to document what happens when there is an EEPROM. Is OF used in preference to the EEPROM? Andrew -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] lan78xx: Read LED modes from Device Tree
On Thu, Apr 12, 2018 at 02:55:35PM +0100, Phil Elwell wrote: > Add support for DT property "microchip,led-modes", a vector of two > cells (u32s) in the range 0-15, each of which sets the mode for one > of the two LEDs. Some possible values are: > > 0=link/activity 1=link1000/activity > 2=link100/activity 3=link10/activity > 4=link100/1000/activity 5=link10/1000/activity > 6=link10/100/activity14=off15=on > > Also use the presence of the DT property to indicate that the > LEDs should be enabled - necessary in the event that no valid OTP > or EEPROM is available. I'm not a fan of this, but at the moment, we don't have anything better. Please follow what mscc does, add a header file for the LED settings. Andrew > > Signed-off-by: Phil Elwell> --- > drivers/net/usb/lan78xx.c | 20 > 1 file changed, 20 insertions(+) > > diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c > index d98397b..ffb483d 100644 > --- a/drivers/net/usb/lan78xx.c > +++ b/drivers/net/usb/lan78xx.c > @@ -2008,6 +2008,7 @@ static int lan78xx_phy_init(struct lan78xx_net *dev) > { > int ret; > u32 mii_adv; > + u32 led_modes[2]; > struct phy_device *phydev; > > phydev = phy_find_first(dev->mdiobus); > @@ -2097,6 +2098,25 @@ static int lan78xx_phy_init(struct lan78xx_net *dev) > (void)lan78xx_set_eee(dev->net, ); > } > > + if (!of_property_read_u32_array(dev->udev->dev.of_node, > + "microchip,led-modes", > + led_modes, ARRAY_SIZE(led_modes))) { > + u32 reg; > + int i; > + > + reg = phy_read(phydev, 0x1d); > + for (i = 0; i < ARRAY_SIZE(led_modes); i++) { > + reg &= ~(0xf << (i * 4)); > + reg |= (led_modes[i] & 0xf) << (i * 4); > + } Please add range checks for led_modes[i] and return -EINVAL if the check fails. Andrew -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] dt-bindings: Document the DT bindings for lan78xx
On Thu, Apr 12, 2018 at 03:10:57PM +0100, Phil Elwell wrote: > Hi Andrew, > > On 12/04/2018 15:04, Andrew Lunn wrote: > > On Thu, Apr 12, 2018 at 02:55:36PM +0100, Phil Elwell wrote: > >> The Microchip LAN78XX family of devices are Ethernet controllers with > >> a USB interface. Despite being discoverable devices it can be useful to > >> be able to configure them from Device Tree, particularly in low-cost > >> applications without an EEPROM or programmed OTP. > >> > >> Document the supported properties in a bindings file, adding it to > >> MAINTAINERS at the same time. > > > > Hi Phil > > > > How you link an OF node to a USB device is not obvious. Could you > > please include either a pointer to some binding documentation, or make > > your example show it. > > Thanks for the feedback. Would you consider this (lifted from the Pi 3B+ > Device Tree) > a sufficient example? Yes, this is good. Thanks Andrew -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] lan78xx: Read initial EEE setting from Device Tree
On Thu, Apr 12, 2018 at 02:55:34PM +0100, Phil Elwell wrote: > Add two new Device Tree properties: > * microchip,eee-enabled - a boolean to enable EEE > * microchip,tx-lpi-timer - time in microseconds to wait after TX goes >idle before entering the low power state >(default 600) Hi Phil This looks wrong. What should happen is that the MAC driver calls phy_init_eee() to find out if the PHY supports EEE. There should be no need to look in device tree. Andrew -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] lan78xx: Read MAC address from DT if present
Hi Phil > - ret = lan78xx_write_reg(dev, RX_ADDRL, addr_lo); > - ret = lan78xx_write_reg(dev, RX_ADDRH, addr_hi); > + mac_addr = of_get_mac_address(dev->udev->dev.of_node); It might be better to use the higher level eth_platform_get_mac_address(). Andrew -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] dt-bindings: Document the DT bindings for lan78xx
On Thu, Apr 12, 2018 at 02:55:36PM +0100, Phil Elwell wrote: > The Microchip LAN78XX family of devices are Ethernet controllers with > a USB interface. Despite being discoverable devices it can be useful to > be able to configure them from Device Tree, particularly in low-cost > applications without an EEPROM or programmed OTP. > > Document the supported properties in a bindings file, adding it to > MAINTAINERS at the same time. Hi Phil How you link an OF node to a USB device is not obvious. Could you please include either a pointer to some binding documentation, or make your example show it. Thanks Andrew -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] lan78xx: Correctly indicate invalid OTP
On Wed, Apr 11, 2018 at 10:59:17AM +0100, Phil Elwell wrote: > lan78xx_read_otp tries to return -EINVAL in the event of invalid OTP > content, but the value gets overwritten before it is returned and the > read goes ahead anyway. Make the read conditional as it should be > and preserve the error code. Hi Phil Do you know that the Fixes: tag should be for this? When did it break? Thanks Andrew -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Question] MFD driver that handles clocks/resets and populates child nodes
On Mon, Apr 02, 2018 at 10:21:01PM +0900, Masahiro Yamada wrote: > 2018-04-02 21:04 GMT+09:00 Andrew Lunn <and...@lunn.ch>: > >> The maintainer of DWC3, Felipe Balbi, requested to > >> split the glue layer driver into small parts such as > >> reset, regulator, phy, etc. > > > > What exactly did Felipe ask for? Did he ask that the patch be split > > up, one patch per reset, regulator, phy etc? > > > Yeah. That is what we understood from his comments. > > > These are the feed-backs from him. > > https://lkml.org/lkml/2018/1/23/298 > https://lkml.org/lkml/2018/1/24/352 > > Are all these resources used just by the DWC3? Or is it a true MFD, > > multiple functions? > > I do not think this is a real MFD. > > This is a DWC3 glue layer, i.e. > a collection of misc registers that control > the DWC3 IP. > > > Just splitting it into small pieces > to use PHY, reset, regulator framework in Linux. > > Of course, the price of this approach > is so cluttered Device Tree, > honestly I do not like it much. This however the correct way to do this. You should have a phy driver, and a regulator driver, and a reset driver. The DWC3 then uses phandles to these drivers. How is the IO map area 65b0 split up. Can you cleanly separate it into sub areas, which do not overlap, so you have a sub-area for the PHY driver, a sub-area for the regulator driver and a sub-area for the reset area? If you can cleanly split it up, you don't need an MFD. If however the registers are in overlapping areas, you do need an MFD. The MFD core provides access to the registers, while its children implement PHY, reset, regulator etc. Andrew -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Question] MFD driver that handles clocks/resets and populates child nodes
> The maintainer of DWC3, Felipe Balbi, requested to > split the glue layer driver into small parts such as > reset, regulator, phy, etc. What exactly did Felipe ask for? Did he ask that the patch be split up, one patch per reset, regulator, phy etc? Are all these resources used just by the DWC3? Or is it a true MFD, multiple functions? Andrew -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] lan78xx: Connect phy early
> @@ -2082,8 +2082,6 @@ static int lan78xx_phy_init(struct lan78xx_net *dev) > > dev->fc_autoneg = phydev->autoneg; > > - phy_start(phydev); > - > netif_dbg(dev, ifup, dev->net, "phy initialised successfully"); > > return 0; > @@ -2512,9 +2510,7 @@ static int lan78xx_open(struct net_device *net) > if (ret < 0) > goto done; > > - ret = lan78xx_phy_init(dev); > - if (ret < 0) > - goto done; > + phy_start(net->phydev); Should the debug message be moved as well? Andrew -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/2] net/usb/ax88179_178a: Adjustments for ax88179_chk_eee()
On Sat, Mar 10, 2018 at 07:22:55PM +0100, SF Markus Elfring wrote: > From: Markus Elfring> Date: Sat, 10 Mar 2018 19:05:45 +0100 > > Two update suggestions were taken into account > from static source code analysis. Hi Markus How about re-writing this driver to use phylib. The whole of ax88179_chk_eee() would then go away, since it is a duplication of phy_init_eee(). Andrew -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] net/{mii,smsc}: Make mii_ethtool_get_link_ksettings and smc_netdev_get_ecmd return void
> diff --git a/drivers/net/cris/eth_v10.c b/drivers/net/cris/eth_v10.c > index da02041..017f48c 100644 > --- a/drivers/net/cris/eth_v10.c > +++ b/drivers/net/cris/eth_v10.c > @@ -1417,10 +1417,9 @@ static int e100_get_link_ksettings(struct net_device > *dev, > { > struct net_local *np = netdev_priv(dev); > u32 supported; > - int err; > > spin_lock_irq(>lock); > - err = mii_ethtool_get_link_ksettings(>mii_if, cmd); > + mii_ethtool_get_link_ksettings(>mii_if, cmd); > spin_unlock_irq(>lock); > > /* The PHY may support 1000baseT, but the Etrax100 does not. */ > @@ -1432,7 +1431,7 @@ static int e100_get_link_ksettings(struct net_device > *dev, > ethtool_convert_legacy_u32_to_link_mode(cmd->link_modes.supported, > supported); > > - return err; > + return 0; > } How far are going planning on going? It seems like *_get_link_ksettings() now all return a useless 0. Do you plan to change ethtool_ops and make if void all the way up? Andrew -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] smsc95xx: Add comments to the registers definition
On Wed, Apr 12, 2017 at 11:24:05AM +0200, Martin Wetterwald wrote: > This chip is used by a lot of embedded devices and also by the Raspberry > Pi 1, 2 & 3 which were created to promote the study of computer > sciences. Students wanting to learn kernel / network device driver > programming through those devices can only rely on the Linux kernel > driver source to make their own. > > This commit adds a lot of comments to the registers definition to expand > the register names. > > Cc: Steve Glendinning <steve.glendinn...@shawell.net> > Cc: Microchip Linux Driver Support <unglinuxdri...@microchip.com> > CC: David Miller <da...@davemloft.net> > Signed-off-by: Martin Wetterwald <mar...@wetterwald.eu> Reviewed-by: Andrew Lunn <and...@lunn.ch> Andrew -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] smsc95xx: Add comments to the registers definition
Hi Martin > @@ -2032,7 +2032,7 @@ static struct sk_buff *smsc95xx_tx_fixup(struct usbnet > *dev, > skb_push(skb, 4); > tx_cmd_b = (u32)(skb->len - 4); > if (csum) > - tx_cmd_b |= TX_CMD_B_CSUM_ENABLE; > + tx_cmd_b |= TX_CMD_B_CSUM_EN; This changed seems a step backwards, ENABLE is much more readable than EN. > > -#define TX_CMD_B_CSUM_ENABLE (0x4000) > -#define TX_CMD_B_ADD_CRC_DISABLE_(0x2000) > -#define TX_CMD_B_DISABLE_PADDING_(0x1000) > -#define TX_CMD_B_PKT_BYTE_LENGTH_(0x07FF) > +#define TX_CMD_B_CSUM_EN (0x4000)/* TX Checksum Enable */ And there is space for ABLE here. Andrew -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC 0/7] phylib MMD accessor cleanups
> Thanks. When I posted this last time around (19th Jan) I mentioned > about marking the old _indirect() accessors with __deprecated - is > that still something we want to do? > > I haven't tested this against net-next yet, so I don't know if there > are any new users of the indirect accessors - going down the deprecated > route would avoid breakage, but means having to submit a patch later to > actually remove them. > > How would people want this handled? Hi Russell We can get patches into net-next very quickly. So i suggest you rebase and resubmit and get it in. If something breaks, we add followup patches to fix it. Andrew -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC 3/7] net: lan78xx: update for phy_(read|write)_mmd_indirect() removal
On Sun, Mar 19, 2017 at 11:00:29AM +, Russell King wrote: > lan78xx appears to use phylib in a rather weird way, accessing the PHY > partly through phylib, and partly by makign direct accesses to it, Hi Russell s/makign/making Andrew -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC 0/7] phylib MMD accessor cleanups
On Sun, Mar 19, 2017 at 10:59:44AM +, Russell King - ARM Linux wrote: > Hi, > > This series cleans up the phylib MMD accessors. We have two accessors > at present, phy_(read|write)_mmd() and phy_(read|write)_mmd_indirect() > > The _indirect methods access the MMD registers via a clause 22 phy, > whereas the non-_indirect methods acess via clause 45 accesses. > > Current PHY-independent parts of phylib (such as EEE) access the MMD > registers via the _indirect methods, which is no good if you have a > clause 45 PHY that doesn't respond to clause 22 accesses. > > In order to make these features available, we need to rework these > accessors such that they can access the MMD registers using a method > dependent on the clause that the PHY conforms with. > > This series of patches does exactly that - we merge the functionality > of the indirect accesses into the clause 45 accessors, and use these > exclusively to access MMD registers. Internally, the new clause > independent MMD accessors indirect via the PHY drivers read_mmd/write_mmd > methods if present, otherwise fall back to using clause 45 accesses if > the PHY is a clause 45 phy, or clause 22 indirect accesses if the PHY > is clause 22. > > Note: confusingly, phy_read_mmd_indirect() vs phy_read_mmd() switches > the order of prtad and devad, which means that converting between the > two is not a simple matter of just replacing the function name. Same > applies for the write methods. Hi Russell This all looks good, apart from the one typo i spotted. Reviewed-by: Andrew Lunn <and...@lunn.ch> Andrew -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/3] Add EHCI support for Armada 37xx
On Thu, Mar 09, 2017 at 12:04:21PM +0100, Gregory CLEMENT wrote: > Hi, > > The EHCI controller in the Armada 37xx SoCs is the one used on many > other mvebu SoCs such as the orion5x, the kirkwood, or the > armada. However, for Armada 37xx an extra initialization step is > needed: this is the purpose of the first patch. > > The second patch allows to build the driver for the ARM64 Armada SoCs. > > The last one enables the EHCI in the device tree. > > This second version takes into account the review from Andrew Lunn and > Thomas Petazzoni. Hi Gregory Thanks for making the changes. Reviewed-by: Andrew Lunn <and...@lunn.ch> Andrew -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] usb: host: Allow to build ehci orion with mvebu SoCs
On Wed, Mar 08, 2017 at 05:24:22PM +0100, Gregory CLEMENT wrote: > The mvebu ARM64 SoCs no more select PLAT_ORION but some of them as the > Armada 37xx use the EHCI orion controller. This patch allow to build > the driver when ARCH_MVEBU is selected. The mvebu ARM64 SoCs no longer selects PLAT_ORION. However Armada 37xx use the Orion EHCI controller. This patch allows the Orion EHCI driver to be built when ARCH_MVEBU is selected. > Signed-off-by: Gregory CLEMENT> --- > drivers/usb/host/Kconfig | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig > index 407d947b34ea..870c42d89298 100644 > --- a/drivers/usb/host/Kconfig > +++ b/drivers/usb/host/Kconfig > @@ -188,7 +188,7 @@ config USB_EHCI_HCD_OMAP > > config USB_EHCI_HCD_ORION > tristate "Support for Marvell EBU on-chip EHCI USB controller" > - depends on USB_EHCI_HCD && PLAT_ORION > + depends on USB_EHCI_HCD && PLAT_ORION || ARCH_MVEBU I don't know the Kconfig language too well. Should this be: > + depends on USB_EHCI_HCD && (PLAT_ORION || ARCH_MVEBU) Andrew -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] usb: orion-echi: Add support for the Armada 3700
Hi Gregory > - Add a new compatoble string for the Armada 3700 SoCs compatible > > - add sbuscfg support for orion usb controller driver. For the SoCs > without hlock, need to program BAWR/BARD/AHBBRST fields in the sbuscfg > register to guarantee the AHB master's burst would not overrun or > underrun the FIFO. > > - the sbuscfg register has to be set after the usb controller reset, > otherwise the value would be overridden to 0. In order to do this, the > reset callback is registered. > > [gregory.clem...@free-electrons.com: - reword commit and comments >- fix checkpatch warning] > Signed-off-by: jinghua> Signed-off-by: Gregory CLEMENT > --- > .../devicetree/bindings/usb/ehci-orion.txt | 4 ++- > drivers/usb/host/ehci-orion.c | 39 > ++ > 2 files changed, 42 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/usb/ehci-orion.txt > b/Documentation/devicetree/bindings/usb/ehci-orion.txt > index 17c3bc858b86..9dfffc9dffec 100644 > --- a/Documentation/devicetree/bindings/usb/ehci-orion.txt > +++ b/Documentation/devicetree/bindings/usb/ehci-orion.txt > @@ -1,7 +1,9 @@ > * EHCI controller, Orion Marvell variants > > Required properties: > -- compatible: must be "marvell,orion-ehci" > +- compatible: could be one of the following must, not could. > + "marvell,orion-ehci" > + "marvell,armada-3700-ehci" > - reg: physical base address of the controller and length of memory mapped >region. > - interrupts: The EHCI interrupt > diff --git a/drivers/usb/host/ehci-orion.c b/drivers/usb/host/ehci-orion.c > index ee8d5faa0194..cf778e166b90 100644 > --- a/drivers/usb/host/ehci-orion.c > +++ b/drivers/usb/host/ehci-orion.c > @@ -47,6 +47,21 @@ > #define USB_PHY_IVREF_CTRL 0x440 > #define USB_PHY_TST_GRP_CTRL 0x450 > > +#define USB_SBUSCFG 0x90 > +#define USB_SBUSCFG_BAWR0x6 > +#define USB_SBUSCFG_BARD0x3 > +#define USB_SBUSCFG_AHBBRST 0x0 These three are all shifts. So i would suggest adding _SHIFT to the end. > + > +/* BAWR = BARD = 3 : Align read/write bursts packets larger than 128 bytes */ > +#define USB_SBUSCFG_BAWR_ALIGN_128B 0x3 > +#define USB_SBUSCFG_BARD_ALIGN_128B 0x3 > +/* AHBBRST = 3 : Align AHB Burst to INCR16 (64 bytes) */ > +#define USB_SBUSCFG_AHBBRST_INCR16 0x3 You can then apply the shift here. > + > +#define USB_SBUSCFG_DEF_VAL ((USB_SBUSCFG_BAWR_ALIGN_128B << > USB_SBUSCFG_BAWR) \ > + | (USB_SBUSCFG_BARD_ALIGN_128B << USB_SBUSCFG_BARD) \ > + | (USB_SBUSCFG_AHBBRST_INCR16 << USB_SBUSCFG_AHBBRST)) and this is then shorted. Andrew -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6] r8152: Add support for setting pass through MAC address on RTL8153-AD
> > It is same, how to handle two network cards which tell us, that they > > have same MAC addresses. > > > > The kernel handles this just fine. In doing this patch I checked to see > what it does in that scenario. Two devices are made. systemd doesn't > rename the second device via the MAC name (eg enxAABBCCDDEEFF). What does you dhcp server do? Does it gives out the same IP address? You then have two interfaces on the same network, with the same MAC address and IP address. Then what happens? Andrew -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6] r8152: Add support for setting pass through MAC address on RTL8153-AD
On Fri, Jun 10, 2016 at 10:51:56PM -0700, David Miller wrote: > From: Mario Limonciello <mario_limoncie...@dell.com> > Date: Tue, 7 Jun 2016 13:22:37 -0500 > > > The RTL8153-AD supports a persistent system specific MAC address. > > This means a device plugged into two different systems with host side > > support will show different (but persistent) MAC addresses. > > > > This information for the system's persistent MAC address is burned in when > > the system HW is built and available under \_SB.AMAC in the DSDT at runtime. > > > > This technology is currently implemented in the Dell TB15 and WD15 Type-C > > docks. More information is available here: > > http://www.dell.com/support/article/us/en/04/SLN301147 > > > > Signed-off-by: Mario Limonciello <mario_limoncie...@dell.com> > > --- > > Changes from v5: > > * Correct return value if hex2bin succesful but invalid ether addr > > Have things calmed down enough now that I can apply this? Hi David I think the code has reaching the level of maturity needed for acceptance. So for the code quality: Reviewed-by: Andrew Lunn <and...@lunn.ch> What is still open is do we want to accept it at all? Do we accept the concept of putting the same MAC address on multiple interfaces at hotplug time? Do we trust BIOS vendors to not keep changing DSDT property name, since it is not standardised? Do we want this at all should be decided by somebody more senior then those passing comments on the code. Andrew -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] r8152: Add support for setting pass through MAC address on RTL8153-AD
> + /* returns _AUXMAC_#AABBCCDDEEFF# */ > + status = acpi_evaluate_object(NULL, "\\_SB.AMAC", NULL, ); > + obj = (union acpi_object *)buffer.pointer; > + if (ACPI_SUCCESS(status)) { > + if (obj->type != ACPI_TYPE_BUFFER || > + obj->string.length != 0x17) { > + pr_warn("r8152: get_passthru_addr: Invalid buffer"); Please don't use pr_warn() when you can use the better alternatives like dev_warn(). Andrew -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] r8152: Add support for setting MAC to system's Auxiliary MAC address
On Thu, Jun 02, 2016 at 07:04:32PM +, mario_limoncie...@dell.com wrote: > > -Original Message- > > From: Andrew Lunn [mailto:and...@lunn.ch] > > Sent: Thursday, June 2, 2016 2:03 PM > > To: Limonciello, Mario <mario_limoncie...@dell.com> > > Cc: gre...@linuxfoundation.org; hayesw...@realtek.com; linux- > > ker...@vger.kernel.org; net...@vger.kernel.org; linux- > > u...@vger.kernel.org; pali.ro...@gmail.com; anthony.w...@canonical.com > > Subject: Re: [PATCH v2] r8152: Add support for setting MAC to system's > > Auxiliary MAC address > > > > > > And you want to check this for all Dell devices? Please be model > > > > specific, I doubt a bunch of Dell servers wants to run this code... > > > > > > > > > > Tracking model specific is really going to turn into a giant list never > > > ending > > list. > > > To drill down more specifically, I can match on chassis too. > > > > Does Dell happen to use its own USB Vendor ID for the USB device in > > the dock? You could go at this problem from the other direction if it > > does have a unique vendor ID. > > > > Andrew > > Unfortunately it's not a Dell specific VID/PID. I'm asking around to find out > if there is something else identifiable about this dock's NIC (maybe that > r8152 > can query). lsusb -v I assume there is a USB hub in the dock, maybe that has a Dell VID? Going one level up the USB tree hierarchy should not be too hard. Andrew -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] r8152: Add support for setting MAC to system's Auxiliary MAC address
> > And you want to check this for all Dell devices? Please be model > > specific, I doubt a bunch of Dell servers wants to run this code... > > > > Tracking model specific is really going to turn into a giant list never > ending list. > To drill down more specifically, I can match on chassis too. Does Dell happen to use its own USB Vendor ID for the USB device in the dock? You could go at this problem from the other direction if it does have a unique vendor ID. Andrew -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] r8152: Add support for setting MAC to system's Auxiliary MAC address
> > > > > + pr_info("r8152: Using system auxiliary MAC address"); > > > > It would be great to write also mac address into that pr_info And since there could be multiple r8152 in the system, it would be good to indicate which of them is having its MAC changed. So netdev_info() or dev_info(). Andrew -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] r8152: Add support for setting MAC to system's Auxiliary MAC address
On Wed, Jun 01, 2016 at 04:50:44PM -0500, Mario Limonciello wrote: > Dell systems with Type-C ports have support for a persistent system > specific MAC address when used with Dell Type-C docks and dongles. > This means a dock plugged into two different systems will show different > (but persistent) MAC addresses. Dell Type-C docks and dongles use the > r8152 driver. > > This information for the system's persistent MAC address is burned in when > the HW is built and avilable under _SB\AMAC in the DSDT at runtime. > > More information about the technology is available here: > http://www.dell.com/support/article/us/en/04/SLN301147 > > Signed-off-by: Mario Limonciello> --- > drivers/net/usb/Kconfig | 1 + > drivers/net/usb/r8152.c | 37 + > 2 files changed, 38 insertions(+) > > diff --git a/drivers/net/usb/Kconfig b/drivers/net/usb/Kconfig > index cdde590..c320930 100644 > --- a/drivers/net/usb/Kconfig > +++ b/drivers/net/usb/Kconfig > @@ -98,6 +98,7 @@ config USB_RTL8150 > config USB_RTL8152 > tristate "Realtek RTL8152/RTL8153 Based USB Ethernet Adapters" > select MII > + depends on ACPI Hi Mario That seems a bit heavy handed. What about ARM or MIPS machines which don't use ACPI but do have USB ports where i could plug in a USB dongle with this chipset. I think it would be better to make use of ACPI if it is available, but don't require it in order the build the driver. Andrew -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [REGRESSION] asix: Lots of asix_rx_fixup() errors and slow transmissions
> In other words, the full-speed hub is restricting the USB to > Ethernet Adaptor to a 12Mbps (half-duplex) bandwidth to support > Ethernet 100Mbps (full-duplex) traffic. That is not going to work > very well because Ethernet frames (perhaps partial Ethernet frames) > need to be discarded within the USB link. If that really is true, the design is broken. I would expect the adaptor to reliably transfer whole frames over USB, and drop whole frames from its receive queue when the USB is congested. TCP is also going to see the USB bottleneck as just like any bottleneck in the network and back off. So TCP streams should not cause major congestion on the USB link. Going over a 12Mbps USB link should be no different to hitting an old Ethernet hub which can only do 10/Half. > Therefore please retest with a working high-speed USB hub or remove > the full-speed USB hub from the test environment and directly > connect the USB to Ethernet Adaptor to the root hub of the USB port. > Then repeat the tests to see whether anything improved. > > In other words, you need to eliminate the dmesg messages saying "not > running at top speed; connect to a high speed hub". I would also suggest testing with the Ethernet at 10/half. You should be able to use Ethtool to set that up. Your USB and Ethernet bandwidth become more equal. If you still see errors, it suggests a protocol implementation error somewhere. Andrew -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] usb: core: add power sequence for USB devices
> So, would you like to accept the generic solution like below: > > - Create a generic power sequence driver, and it will be probed > according to compatible string at device tree. At its probe, > we can create a power sequence structure, and let this structure > as the private data for this power sequence device. I'm not sure a separate driver is required. Why not consider it more like pinctrl properties? They are listed in the devices node. Have the bus enumerate code first walk all children and run their on sequence. Bus shutdown would again walk the children and run the off sequence. Andrew -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] usb: core: add power sequence for USB devices
On Fri, Mar 04, 2016 at 10:02:42AM +0800, Peter Chen wrote: > On Thu, Mar 03, 2016 at 02:54:55PM -0600, Rob Herring wrote: > > On Thu, Mar 3, 2016 at 4:01 AM, Peter Chenwrote: > > > Some hard-wired USB devices need to do power sequence to let the > > > device work normally, the typical power sequence like: enable USB > > > PHY clock, toggle reset pin, etc. But current Linux USB driver > > > lacks of such code to do it, it may cause some hard-wired USB devices > > > works abnormal or can't be recognized by controller at all. > > > > > > In this patch, it will do power on sequence at hub's probe for all > > > devices under this hub (includes root hub) if this device is described > > > at dts and there is a phandle "usb-pwrseq" for it. At hub_disconnect, > > > it will do power off sequence. > > > > > > Signed-off-by: Peter Chen > > > --- > > > .../devicetree/bindings/usb/usb-device.txt | 41 +- > > > drivers/usb/core/Makefile | 2 +- > > > drivers/usb/core/hub.c | 32 + > > > drivers/usb/core/pwrseq.c | 149 > > > + > > > include/linux/usb/of.h | 10 ++ > > > 5 files changed, 232 insertions(+), 2 deletions(-) > > > create mode 100644 drivers/usb/core/pwrseq.c > > > > > > diff --git a/Documentation/devicetree/bindings/usb/usb-device.txt > > > b/Documentation/devicetree/bindings/usb/usb-device.txt > > > index 1c35e7b..c7a298c 100644 > > > --- a/Documentation/devicetree/bindings/usb/usb-device.txt > > > +++ b/Documentation/devicetree/bindings/usb/usb-device.txt > > > @@ -13,8 +13,37 @@ Required properties: > > > - reg: the port number which this device is connecting to, the range > > >is 1-31. > > > > > > +Optional properties: > > > +- usb-pwrseq: the power sequence handler which need to do before this USB > > > + device can work. > > > +- clocks: the input clock for USB device. > > > +- clock-frequency: the frequency for device's clock. > > > +- reset-gpios: Should specify the GPIO for reset. > > > +- reset-duration-us: the duration in microsecond for assert reset signal. > > > > So we sorted out how to describe USB devices in DT, but now we don't > > use it? > > We only know USB device after USB bus finds it, but without power > on sequence, the USB bus can't find it. Not really true. Device tree says it exists, you just cannot see it yet. The fact it is in device tree means it is soldered on the board and really is there. So when the host controller enumerates the bus, it should run the power sequence of all child nodes to make them appear. Andrew -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] usb: chipidea: host: let the hcd know's parent device node
> > > diff --git a/drivers/usb/chipidea/host.c b/drivers/usb/chipidea/host.c > > > index 053bac9..55120ef 100644 > > > --- a/drivers/usb/chipidea/host.c > > > +++ b/drivers/usb/chipidea/host.c > > > @@ -109,15 +109,25 @@ static int host_start(struct ci_hdrc *ci) > > > struct ehci_hcd *ehci; > > > struct ehci_ci_priv *priv; > > > int ret; > > > + struct device *dev = ci->dev; > > > > > > - if (usb_disabled()) > > > + if (usb_disabled() || !dev) > > > return -ENODEV; > > > > > > - hcd = usb_create_hcd(_ehci_hc_driver, ci->dev, dev_name(ci->dev)); > > > + /* > > > + * USB Core will try to get child node under roothub, > > > + * but chipidea core has no of_node, and the child node > > > + * for controller is located at glue layer's node which > > > + * is chipidea core's parent. > > > + */ > > > + if (dev->parent && dev->parent->of_node) > > > + dev->of_node = dev->parent->of_node; > > > > Is this a good idea? Two devices with the same of_node? > > > > This is only for chipidea driver whose host controller device > doesn't have entry at dts, but other host controller driver which > supports device tree should have its entry at dts. > > > I know the networking code assumes of_node values are unique, and uses > > it to find a device. Are you 100% sure the USB code does not make this > > assumption. > > > > The controller device is the root for USB device, the common > USB code will not touch its glue layer device (controller's parent). I'm just thinking about code like: of_find_spi_master_by_node(), of_find_net_device_by_node(), of_find_backlight_by_node(), etc. If somebody was to implement an of_find_usb_host_by_node() are you 100% sure the right node will be found? This seems like a bug waiting to happen. Andrew -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] usb: chipidea: host: let the hcd know's parent device node
On Thu, Mar 03, 2016 at 06:01:15PM +0800, Peter Chen wrote: > From: Peter Chen> > Since the hcd (chipidea core device) has no device node, so > if we want to describe the child node under the hcd, we had > to put it under its parent's node (glue layer device), and > in the code, we need to let the hcd knows glue layer's code, > then the USB core can handle this node. > > Signed-off-by: Peter Chen > --- > drivers/usb/chipidea/host.c | 18 ++ > 1 file changed, 14 insertions(+), 4 deletions(-) > > diff --git a/drivers/usb/chipidea/host.c b/drivers/usb/chipidea/host.c > index 053bac9..55120ef 100644 > --- a/drivers/usb/chipidea/host.c > +++ b/drivers/usb/chipidea/host.c > @@ -109,15 +109,25 @@ static int host_start(struct ci_hdrc *ci) > struct ehci_hcd *ehci; > struct ehci_ci_priv *priv; > int ret; > + struct device *dev = ci->dev; > > - if (usb_disabled()) > + if (usb_disabled() || !dev) > return -ENODEV; > > - hcd = usb_create_hcd(_ehci_hc_driver, ci->dev, dev_name(ci->dev)); > + /* > + * USB Core will try to get child node under roothub, > + * but chipidea core has no of_node, and the child node > + * for controller is located at glue layer's node which > + * is chipidea core's parent. > + */ > + if (dev->parent && dev->parent->of_node) > + dev->of_node = dev->parent->of_node; Is this a good idea? Two devices with the same of_node? I know the networking code assumes of_node values are unique, and uses it to find a device. Are you 100% sure the USB code does not make this assumption. Andrew -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] asix: do not free array priv->mdio->irq
On Thu, Mar 03, 2016 at 01:27:56PM +, Colin King wrote: > From: Colin Ian King <colin.k...@canonical.com> > > Used to be allocated and required freeing, but now > priv->mdio->irq is now a fixed sized array and should no longer be > free'd. > > Issue detected using static analysis with CoverityScan > > Fixes: e7f4dc3536a400 ("mdio: Move allocation of interrupts into core") > Signed-off-by: Colin Ian King <colin.k...@canonical.com> Reviewed-by: Andrew Lunn <and...@lunn.ch> Thanks Andrew > --- > drivers/net/usb/ax88172a.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/drivers/net/usb/ax88172a.c b/drivers/net/usb/ax88172a.c > index 224e7d8..cf77f2d 100644 > --- a/drivers/net/usb/ax88172a.c > +++ b/drivers/net/usb/ax88172a.c > @@ -134,7 +134,6 @@ static void ax88172a_remove_mdio(struct usbnet *dev) > > netdev_info(dev->net, "deregistering mdio bus %s\n", priv->mdio->id); > mdiobus_unregister(priv->mdio); > - kfree(priv->mdio->irq); > mdiobus_free(priv->mdio); > } > > -- > 2.7.0 > -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [ehci-orion] ETIMEOUT with ehci_setup()'s ehci_halt()
One final update on this topic: the kernel config was missing the CONFIG_USB_EHCI_ROOT_HUB_TT option. It turns out that the USB IP embeded a Transaction Translator and there is a check in ehci_halt() that aborts the routine in case of a Transaction Translator in the ehci controller. The check however only works in the above option is enabled. Maybe it would be worth detecting this and issuing a warning? Andrew -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [ehci-orion] ETIMEOUT with ehci_setup()'s ehci_halt()
On Fri, Jun 05, 2015 at 04:34:54PM +0200, Valentin Longchamp wrote: Hello, I am currently bringing up the USB 2.0 Host controller of the Bobcat's (98DX4122 Marvell switch) internal kirkwood CPU on a variation of Keymile's km_kirwood hardware (kirkwood-km_kirkwood.dts). When the driver registers its hcd, it fails with a timemout as it can be seen in the below log: root@kmcoge5un:~# dmesg -n 8 root@kmcoge5un:~# insmod ehci-orion.ko ehci-orion: EHCI orion driver Initializing Orion-SoC USB Host Controller orion-ehci f105.ehci: EHCI Host Controller orion-ehci f105.ehci: new USB bus registered, assigned bus number 1 orion-ehci f105.ehci: reset hcs_params 0x10011 dbg=0 ind cc=0 pcc=0 ordered ports=1 orion-ehci f105.ehci: reset hcc_params 0006 thresh 0 uframes 256/512/1024 park orion-ehci f105.ehci: park 0 orion-ehci f105.ehci: reset command 0080002 (park)=0 ithresh=8 period=1024 Reset HALT orion-ehci f105.ehci: can't setup orion-ehci f105.ehci: USB bus 1 deregistered orion-ehci f105.ehci: init f105.ehci fail, -110 orion-ehci: probe of f105.ehci failed with error -110 This is very similar to this problem, except that it always happens: https://lkml.org/lkml/2012/6/4/381 I have cornered it out to the last handshake() call of the ehci_halt() call, that should set the controller in the HALT state. If I comment this handshake() call out, the registration is fine and the controller then looks OK (I don't have a hardware with a real USB bus to further test it yet). My current idea is that maybe a reset or an initialization may be missing, but I wanted to ask if anybody already has seen a similar behavior with the various hardware platform supported by ehci-orion ? Hi Valentin I don't remember seeing any problems like this on any kirkwood devices. Do you know what phy the 98DX4122 uses? The ehci-orion.c has some code for handling the Orion5X USB phy. I also think u-boot might also have some code. Sebastian might know more, i think he implemented USB support for barebox. Andrew -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 0/3] Enable connecting DSA-based switch to the USB RMII interface.
On Wed, Apr 22, 2015 at 04:14:33PM +, Jan Kaisrlik wrote: 2015-04-21 17:51 GMT+00:00 Florian Fainelli f.faine...@gmail.com: On 21/04/15 10:39, Andrew Lunn wrote: I would however say that sysfs is the wrong API. The linux network stack uses netlink for most configuration activities. So i would suggest adding a netlink binding to DSA, and place the code in net/dsa/, not within an MDIO driver. I suppose we could do that, but that sounds like a pretty radical change in how DSA is currently configured (that is statically at boot time), part in order to allow booting from DSA-enabled network devices (e.g: nfsroot). We would keep both DT and platform device. But statically at boot does not work for a USB hotpluggable switch! Is the switch really hotpluggable, or it is the USB-Ethernet adapter connecting to it? If the former, then I agree, if not, I would imagine that there is nothing that prevents creating the switch device first, and wait for its master_netdev to show up later before it starts doing anything useful? -- Florian Thank you for your quick and helpful answers. The goal of this project is to extend embeded modules without integreted MII to add possibility to connect ethernet switch. Current version of switch is hotplugable but this feature is not required. In my humble opinion, hotplugable switch seems to be pretty interesting idea. Hi Jan Thanks for the extra information. I don't know this USB device. Can you change the product:vendor ID? Could you imply from the USB product:vendor ID what the DSA configuration is? So have a wrapper driver around the asix driver which installs a dsa platform device and then instantiates the asix driver? That eliminates all your DSA changes, no need for a user space API, etc. Andrew -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 0/3] Enable connecting DSA-based switch to the USB RMII interface.
I would however say that sysfs is the wrong API. The linux network stack uses netlink for most configuration activities. So i would suggest adding a netlink binding to DSA, and place the code in net/dsa/, not within an MDIO driver. I suppose we could do that, but that sounds like a pretty radical change in how DSA is currently configured (that is statically at boot time), part in order to allow booting from DSA-enabled network devices (e.g: nfsroot). We would keep both DT and platform device. But statically at boot does not work for a USB hotpluggable switch! Andrew -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 0/3] Enable connecting DSA-based switch to the USB RMII interface.
My goal in reworking this weird DSA device/driver model is that you could just register your switch devices as an enhanced phy_driver/spi_driver/pci_driver etc..., such that libphy-ready drivers could just take advantage of that when they scan/detect their MDIO buses and find a switch. We are not quite there yet, but some help could be welcome, here are the WIP patches (tested with platform_driver only so far): We are hijacking another thread, but... I don't understand you here. Who calls dsa_switch_register()? I know of a board coming soon which has three switch chips on it. There is one MDIO device in the Soc, but there is an external MDIO multiplexor controlled via gpio lines, such that each switch has its own MDIO bus. The DT binding does not support this currently, but the underlying data structures do. How do you envisage dsa_switch_register() to work in such a setup? Thanks Andrew -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 0/3] Enable connecting DSA-based switch to the USB RMII interface.
Hi Jan Interesting work, but i think the architecture is wrong. DSA needs an Ethernet device, an MDIO bus, and information about ports on the switch. The MDIO bus and the Ethernet need no knowledge of DSA. So putting your DSA configuration code in the MDIO driver is wrong. The problem you have is where the put the configuration data. There are the currently two choices, using a platform driver, which you can find some examples of in arch/arm/mach-orion5x, or via device tree. Or you need a new method. Part of your problem is hotplug, since you have a USB device, and no stable names for the ethernet device nor the MDIO device. Your hardware is not fixed, you could hang any switch off the USB device. So it does sound like you need a user space API. I would however say that sysfs is the wrong API. The linux network stack uses netlink for most configuration activities. So i would suggest adding a netlink binding to DSA, and place the code in net/dsa/, not within an MDIO driver. Device tree overlays might be a solution, if you can dynamically load a blob as part of a USB hotplug event. What makes it easier is that both the Ethernet device and MDIO bus are on the same USB device, so all your phandles are within the blob. What is your long term goal? Is this just a development tool? Are you thinking of making a product which integrates both the switch and the USB ethernet onto a USB dongle? This could also change the architecture, since it makes the configuration more fixed. Andrew -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/3] ARM: mvebu: Enable XHCI on the Armada 385 AP
On Tue, Jan 20, 2015 at 09:30:28PM +0100, Maxime Ripard wrote: Hi Andrew, On Mon, Jan 19, 2015 at 10:35:07PM +0100, Andrew Lunn wrote: On Mon, Jan 19, 2015 at 02:01:11PM +0100, Maxime Ripard wrote: Hi all, This serie enables the Armada 385 AP XHCI controller. Since the controller uses a GPIO-controlled VBUS, we used the phy-generic driver, and made the needed additions to the xhci-plat driver to retrieve a USB phy. Unfortunately, some glitches were also found along the way, mostly because of the probe deferring that was introduced by this phy retrieval. Since the introduction of the Armada 38x support in 3.16, the driver was attempting to write into registers while the clock wasn't enabled yet. This was working because the bootloader left it enabled, but in the case of a deferred probing, the clock would have been disabled by the error path of our driver, and this would fail. This should go in 3.19, and any stable kernel for 3.16+. The two patches remaining are regular patches, and are aimed at 3.20. The last patch depend on my previous serie to introduce support for the the A385 AP board. Hi Maxime I assume you want me to take 3/3? Any other route is not simple, since this file only exists in mvebu/dt and maybe a staging branch of arm-soc. What route do you think the other patches will take? There should be no merge dependency, but merging the third patch alone will probably result on a boot breakage. I don't think it really matters though, since this is a new board, so I guess it can go through the USB-PHY tree. Hi Maxime Humm, maybe i'm wrong, but i think arch/arm/boot/dts/armada-385-db-ap.dts only exists in the mvebu tree? At least, i don't see it here: https://git.kernel.org/cgit/linux/kernel/git/balbi/usb.git/tree/arch/arm/boot/dts?h=next Andrew -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/3] ARM: mvebu: Enable XHCI on the Armada 385 AP
On Mon, Jan 19, 2015 at 02:01:11PM +0100, Maxime Ripard wrote: Hi all, This serie enables the Armada 385 AP XHCI controller. Since the controller uses a GPIO-controlled VBUS, we used the phy-generic driver, and made the needed additions to the xhci-plat driver to retrieve a USB phy. Unfortunately, some glitches were also found along the way, mostly because of the probe deferring that was introduced by this phy retrieval. Since the introduction of the Armada 38x support in 3.16, the driver was attempting to write into registers while the clock wasn't enabled yet. This was working because the bootloader left it enabled, but in the case of a deferred probing, the clock would have been disabled by the error path of our driver, and this would fail. This should go in 3.19, and any stable kernel for 3.16+. The two patches remaining are regular patches, and are aimed at 3.20. The last patch depend on my previous serie to introduce support for the the A385 AP board. Hi Maxime I assume you want me to take 3/3? Any other route is not simple, since this file only exists in mvebu/dt and maybe a staging branch of arm-soc. What route do you think the other patches will take? Thanks Andrew -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 01/12] reset: add the Berlin reset controller driver
On Wed, Jul 16, 2014 at 10:25:55AM +0200, Antoine Ténart wrote: Add a reset controller for Marvell Berlin SoCs which is used by the USB PHYs drivers (for now). Signed-off-by: Antoine Ténart antoine.ten...@free-electrons.com Signed-off-by: Sebastian Hesselbarth sebastian.hesselba...@gmail.com Acked-by: Philipp Zabel p.za...@pengutronix.de --- drivers/reset/Makefile | 1 + drivers/reset/reset-berlin.c | 131 +++ 2 files changed, 132 insertions(+) create mode 100644 drivers/reset/reset-berlin.c diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile index 60fed3d7820b..157d421f755b 100644 --- a/drivers/reset/Makefile +++ b/drivers/reset/Makefile @@ -1,4 +1,5 @@ obj-$(CONFIG_RESET_CONTROLLER) += core.o obj-$(CONFIG_ARCH_SOCFPGA) += reset-socfpga.o +obj-$(CONFIG_ARCH_BERLIN) += reset-berlin.o obj-$(CONFIG_ARCH_SUNXI) += reset-sunxi.o obj-$(CONFIG_ARCH_STI) += sti/ diff --git a/drivers/reset/reset-berlin.c b/drivers/reset/reset-berlin.c new file mode 100644 index ..7b894047a81d --- /dev/null +++ b/drivers/reset/reset-berlin.c @@ -0,0 +1,131 @@ +/* + * Copyright (C) 2014 Marvell Technology Group Ltd. + * + * Antoine Ténart antoine.ten...@free-electrons.com + * Sebastian Hesselbarth sebastian.hesselba...@gmail.com + * + * This file is licensed under the terms of the GNU General Public + * License version 2. This program is licensed as is without any + * warranty of any kind, whether express or implied. + */ + +#include linux/delay.h +#include linux/io.h +#include linux/module.h +#include linux/of.h +#include linux/of_address.h +#include linux/platform_device.h +#include linux/reset-controller.h +#include linux/slab.h +#include linux/types.h + +#define BERLIN_MAX_RESETS32 + +#define to_berlin_reset_priv(p) \ + container_of((p), struct berlin_reset_priv, rcdev) + +struct berlin_reset_priv { + void __iomem*base; + unsigned intsize; + struct reset_controller_dev rcdev; +}; + +static int berlin_reset_reset(struct reset_controller_dev *rcdev, + unsigned long id) +{ + struct berlin_reset_priv *priv = to_berlin_reset_priv(rcdev); + int offset = id 8; + int mask = BIT(id 0xff); nit: The parameter to BIT() should be 0-31. So you want to with 5. Given your xlate function, it should never happen that it is greater than 31. Hence it is only a nit. If you need to respin, you can change this, but don't bother to respin just because of this. Andrew -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/5] Documentation: dt-bindings: document the Armada 375 USB cluster binding
On Fri, May 23, 2014 at 02:54:02PM +0530, Kishon Vijay Abraham I wrote: Hi, On Friday 16 May 2014 09:52 PM, Gregory CLEMENT wrote: Armada 375 comes with an USB2 host and device controller and an USB3 controller. The USB cluster control register allows to manage common features of both USB controllers. This commit adds the Device Tree binding documentation for this piece of hardware. Pls re-order so that the Documentation patch comes before the driver patch.. Signed-off-by: Gregory CLEMENT gregory.clem...@free-electrons.com --- .../bindings/phy/armada-375-usb-phy-cluster.txt | 20 1 file changed, 20 insertions(+) create mode 100644 Documentation/devicetree/bindings/phy/armada-375-usb-phy-cluster.txt diff --git a/Documentation/devicetree/bindings/phy/armada-375-usb-phy-cluster.txt b/Documentation/devicetree/bindings/phy/armada-375-usb-phy-cluster.txt simpler file name? armada-phy? Armada is a collection of families of SoCs. This driver is very specific to one SoC, in one family of SoCs. Do you want one .txt file per driver, or can we combine binding documentations into one file? There should already be a mvebu-phy.txt, which contains the sata phy usable on some of the Armada SoC families. This binding could be appended to it. Andrew -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/5] Documentation: dt-bindings: document the Armada 375 USB cluster binding
Do you want one .txt file per driver, or can we combine binding documentations into one file? There should already be a mvebu-phy.txt, which contains the sata phy usable on some of the Armada SoC families. This binding could be appended to it. Ah. Humm, well! It seems the patch adding the mvebu-phy.txt fell through a crack when adding the driver. Kishon could you take http://www.spinics.net/lists/devicetree/msg17018.html into your tree? It should of been taken the same time you took the actual driver, http://www.spinics.net/lists/devicetree/msg17011.html into your tree. I can resend that one missing patch if you want. Andrew -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/5] Documentation: dt-bindings: document the Armada 375 USB cluster binding
On Fri, May 23, 2014 at 07:22:48PM +0530, Kishon Vijay Abraham I wrote: hI, On Friday 23 May 2014 07:06 PM, Andrew Lunn wrote: Do you want one .txt file per driver, or can we combine binding documentations into one file? There should already be a mvebu-phy.txt, which contains the sata phy usable on some of the Armada SoC families. This binding could be appended to it. Ah. Humm, well! It seems the patch adding the mvebu-phy.txt fell through a crack when adding the driver. Kishon could you take http://www.spinics.net/lists/devicetree/msg17018.html into your tree? It should of been taken the same time you took the actual driver, http://www.spinics.net/lists/devicetree/msg17011.html into your tree. I can resend that one missing patch if you want. yes please. But it's already too late to go in the next merge window. Gregory, could you pick it up and append your 375 binding to it? We can avoid merge conflicts that way. Thanks Andrew -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 02/20] usb: ehci-orion: Add the optional PHY support
On Wed, May 07, 2014 at 11:40:06AM +0200, Thomas Petazzoni wrote: Dear Andrew Lunn, On Tue, 6 May 2014 15:33:41 +0200, Andrew Lunn wrote: + priv-phy = devm_phy_get(pdev-dev, usb); + if (!IS_ERR(priv-phy)) { + err = phy_init(priv-phy); + if (err) + goto err2; + + err = phy_power_on(priv-phy); + if (err) + goto err3; + } Hi Gregory What about EPROBE_DEFERRED? In v4 (to be submitted soon), I've changed this to: priv-phy = devm_phy_optional_get(pdev-dev, usb); if (IS_ERR(priv-phy)) { err = PTR_ERR(priv-phy); goto err_phy_get; } else { err = phy_init(priv-phy); if (err) goto err_phy_init; err = phy_power_on(priv-phy); if (err) goto err_phy_power_on; } Thanks to devm_phy_optional_get(), the fact of not having a PHY in the DT is not considered an error. So on any error from devm_phy_optional_get() (including -EPROBE_DEFER), we simply bail out. Does this looks good? Hi Thomas That looks good. To avoid the SATA phy problems we had last time, i would like to test this on a few systems. Please could you let me know what branch v4 is in when it is ready. Thanks Andrew -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 02/20] usb: ehci-orion: Add the optional PHY support
On Tue, May 06, 2014 at 02:13:57AM +0200, Gregory CLEMENT wrote: This commit allows to use the PHY provided through the device tree. It will be useful for the Armada 375 SoCs. if no PHY is provided then the behavior of the driver is unchanged. Signed-off-by: Gregory CLEMENT gregory.clem...@free-electrons.com --- drivers/usb/host/ehci-orion.c | 25 +++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/drivers/usb/host/ehci-orion.c b/drivers/usb/host/ehci-orion.c index d6c19c37c76b..f6d9eb2e33cd 100644 --- a/drivers/usb/host/ehci-orion.c +++ b/drivers/usb/host/ehci-orion.c @@ -15,6 +15,7 @@ #include linux/clk.h #include linux/platform_data/usb-ehci-orion.h #include linux/of.h +#include linux/phy/phy.h #include linux/of_device.h #include linux/of_irq.h #include linux/usb.h @@ -46,6 +47,7 @@ struct orion_ehci_hcd { struct clk *clk; + struct phy *phy; }; static const char hcd_name[] = ehci-orion; @@ -224,6 +226,18 @@ static int ehci_orion_drv_probe(struct platform_device *pdev) if (!IS_ERR(priv-clk)) clk_prepare_enable(priv-clk); + + priv-phy = devm_phy_get(pdev-dev, usb); + if (!IS_ERR(priv-phy)) { + err = phy_init(priv-phy); + if (err) + goto err2; + + err = phy_power_on(priv-phy); + if (err) + goto err3; + } Hi Gregory What about EPROBE_DEFERRED? Andrew -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 08/20] ARM: mvebu: Add Device Tree description of xHCI hosts on Armada 38x
On Tue, May 06, 2014 at 02:14:03AM +0200, Gregory CLEMENT wrote: The Marvell Armada 38x SoCs contains two xHCI host. This commit adds the Device Tree description of those interfaces at the SoC level, and also enables the two USB3 ports on the Armada 385 DB platform and one USB3 port on the Armada 385 RD platform. Signed-off-by: Gregory CLEMENT gregory.clem...@free-electrons.com --- arch/arm/boot/dts/armada-385-db.dts | 8 arch/arm/boot/dts/armada-385-rd.dts | 4 arch/arm/boot/dts/armada-38x.dtsi | 17 + 3 files changed, 29 insertions(+) diff --git a/arch/arm/boot/dts/armada-385-db.dts b/arch/arm/boot/dts/armada-385-db.dts index 6828d77696a6..d5db1466da82 100644 --- a/arch/arm/boot/dts/armada-385-db.dts +++ b/arch/arm/boot/dts/armada-385-db.dts @@ -101,6 +101,14 @@ reg = 0x100 0x3f00; }; }; + + usb3@f { + status = okay; + }; + + usb3@f8000 { + status = okay; + }; }; pcie-controller { diff --git a/arch/arm/boot/dts/armada-385-rd.dts b/arch/arm/boot/dts/armada-385-rd.dts index 45250c88814b..a505fe94ff37 100644 --- a/arch/arm/boot/dts/armada-385-rd.dts +++ b/arch/arm/boot/dts/armada-385-rd.dts @@ -77,6 +77,10 @@ reg = 1; }; }; + + usb3@f { + status = okay; + }; }; pcie-controller { diff --git a/arch/arm/boot/dts/armada-38x.dtsi b/arch/arm/boot/dts/armada-38x.dtsi index a064f59da02d..5913ce1cc601 100644 --- a/arch/arm/boot/dts/armada-38x.dtsi +++ b/arch/arm/boot/dts/armada-38x.dtsi @@ -355,6 +355,23 @@ clocks = coredivclk 0; status = disabled; }; + + usb3@f { + compatible = marvell,armada-380-xhci; + reg = 0xf 0x3fff,0xf4000 0x3fff; 0x3fff? I would expect that to be 0x4000. Andrew -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 17/20] phy: Add support for USB cluster on the Armada 375 SoC
On Tue, May 06, 2014 at 02:14:12AM +0200, Gregory CLEMENT wrote: The Armada 375 SoC comes with an USB2 host and device controller and an USB3 controller. The USB cluster control register allows to manage common features of both USB controllers. It uses the generic PHY framework Signed-off-by: Gregory CLEMENT gregory.clem...@free-electrons.com --- drivers/phy/Kconfig | 5 ++ drivers/phy/Makefile | 1 + drivers/phy/phy-armada375-usb2.c | 154 +++ 3 files changed, 160 insertions(+) create mode 100644 drivers/phy/phy-armada375-usb2.c diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig index 3bb05f17b9b4..cdf3e2c24e3a 100644 --- a/drivers/phy/Kconfig +++ b/drivers/phy/Kconfig @@ -15,6 +15,11 @@ config GENERIC_PHY phy users can obtain reference to the PHY. All the users of this framework should select this config. +config ARMADA375_USBCLUSTER_PHY + def_bool y + depends on OF + select GENERIC_PHY + config PHY_EXYNOS_MIPI_VIDEO tristate S5P/EXYNOS SoC series MIPI CSI-2/DSI PHY driver depends on HAS_IOMEM diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile index 2faf78edc864..47d5a86807b6 100644 --- a/drivers/phy/Makefile +++ b/drivers/phy/Makefile @@ -3,6 +3,7 @@ # obj-$(CONFIG_GENERIC_PHY)+= phy-core.o +obj-$(CONFIG_ARMADA375_USBCLUSTER_PHY) += phy-armada375-usb2.o obj-$(CONFIG_BCM_KONA_USB2_PHY) += phy-bcm-kona-usb2.o obj-$(CONFIG_PHY_EXYNOS_DP_VIDEO)+= phy-exynos-dp-video.o obj-$(CONFIG_PHY_EXYNOS_MIPI_VIDEO) += phy-exynos-mipi-video.o diff --git a/drivers/phy/phy-armada375-usb2.c b/drivers/phy/phy-armada375-usb2.c new file mode 100644 index ..8bbac45e72c8 --- /dev/null +++ b/drivers/phy/phy-armada375-usb2.c @@ -0,0 +1,154 @@ +/* + * USB cluster support for Armada 375 platform. + * + * Copyright (C) 2014 Marvell + * + * Gregory CLEMENT gregory.clem...@free-electrons.com + * + * This file is licensed under the terms of the GNU General Public + * License version 2 or later. This program is licensed as is + * without any warranty of any kind, whether express or implied. + * + * Armada 375 comes with an USB2 host and device controller and an + * USB3 controller. The USB cluster control register allows to manage + * common features of both USB controller. + */ + +#include linux/init.h +#include linux/io.h +#include linux/kernel.h +#include linux/module.h +#include linux/of_address.h +#include linux/phy/phy.h +#include linux/platform_device.h +#include linux/slab.h + +#define USB2_PHY_CONFIG_ENABLE BIT(0) /* active low */ + +/* the USB cluster allow to choose between two PHYs*/ +#define NB_PHY 2 + +enum { + PHY_USB2 = 0, + PHY_USB3 = 1, +}; + +struct armada375_cluster_phy { + struct phy *phy; + void __iomem *reg; + bool enable; + bool use_usb3; Hi Gregory nit: How about using the enum you just defined? +}; + +struct armada375_cluster_phy usb_cluster_phy[NB_PHY]; + +static int armada375_usb_phy_init(struct phy *phy) +{ + struct armada375_cluster_phy *cluster_phy = phy_get_drvdata(phy); + u32 reg; + + if (cluster_phy-enable) { + reg = readl(cluster_phy-reg); + if (cluster_phy-use_usb3) + reg |= USB2_PHY_CONFIG_ENABLE; + else + reg = ~USB2_PHY_CONFIG_ENABLE; + writel(reg, cluster_phy-reg); + + return 0; + } else { + return -ENODEV; + } +} + +static struct phy_ops armada375_usb_phy_ops = { + .init = armada375_usb_phy_init, + .owner = THIS_MODULE, +}; + +static struct phy *armada375_usb_phy_xlate(struct device *dev, + struct of_phandle_args *args) +{ + if (WARN_ON(args-args[0] = NB_PHY)) + return ERR_PTR(-ENODEV); + + return usb_cluster_phy[args-args[0]].phy; +} + +static int armada375_usb_phy_probe(struct platform_device *pdev) +{ + struct device *dev = pdev-dev; + struct phy *phy; + struct device_node *np = dev-of_node; + struct phy_provider *phy_provider; + void __iomem *usb_cluster_base; + struct device_node *xhci_node; + int i; + + usb_cluster_base = of_iomap(np, 0); devm_ API? Check the return value for an error? + BUG_ON(!usb_cluster_base); + + for (i = 0; i NB_PHY; i++) { + phy = devm_phy_create(dev, armada375_usb_phy_ops, NULL); + if (IS_ERR(phy)) + dev_err(dev, failed to create PHY n%d\n, i); + + usb_cluster_phy[i].phy = phy; + usb_cluster_phy[i].reg = usb_cluster_base; + usb_cluster_phy[i].enable = false; + phy_set_drvdata(phy, usb_cluster_phy[i]); + } + + usb_cluster_phy[PHY_USB2].use_usb3 = false; +
Re: [PATCH v2 02/18] usb: host: xhci-plat: Add clocks support
On Fri, Apr 25, 2014 at 04:07:00PM +0200, Gregory CLEMENT wrote: Some platform (such as the Armada 38x ones) can gate the clock of their USB controller. This patch add the support for the clock, by enabling them during probe and disabling them on remove. As not all platforms have clock support then enabling and disabling the clocks have been placed in separate functions. Then if the clocks are not supported we still can use the same calls, and there is no Signed-off-by: Gregory CLEMENT gregory.clem...@free-electrons.com --- drivers/usb/host/xhci-plat.c | 52 ++-- 1 file changed, 50 insertions(+), 2 deletions(-) diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c index f5351af4b2c5..bb5d563f729c 100644 --- a/drivers/usb/host/xhci-plat.c +++ b/drivers/usb/host/xhci-plat.c @@ -11,6 +11,7 @@ * version 2 as published by the Free Software Foundation. */ +#include linux/clk.h #include linux/dma-mapping.h #include linux/module.h #include linux/of.h @@ -85,6 +86,42 @@ static const struct hc_driver xhci_plat_xhci_driver = { .bus_resume = xhci_bus_resume, }; +#if defined(CONFIG_HAVE_CLK) Hi Gregory You probably don't need to do this conditional on CONFIG_HAVE_CLK. There are stub functions in clk.h for when CONFIG_HAVE_CLK is not defined. So the compiler knows devm_clk_get() will return NULL, it can evaluate IS_ERR is false, clk_prepare() returns 0 and so the whole function collapses to return 0; +static int try_enable_clk(struct platform_device *pdev) +{ + struct clk *clk = devm_clk_get(pdev-dev, NULL); + + /* Not all platforms have a clk so it is not an error if the clock +does not exists. */ + if (!IS_ERR(clk)) + if (clk_prepare_enable(clk)) + return -ENODEV; Is ENODEV the correct error code? It would probably be better to return the return value of clk_prepare_enable() + return 0; +} -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 14/18] ARM: mvebu: Add support for USB cluster on the Armada 375 SoC
+* We can't use usb2 and usb3 in the same time, so let's +* disbale usb2 and complain about it to the user askinf typos: disable, asking And it should be at the same time, not in. Andrew -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 05/18] ARM: mvebu: Add Device Tree description of xHCI hosts on Armada 38x
On Fri, Apr 25, 2014 at 04:07:03PM +0200, Gregory CLEMENT wrote: The Marvell Armada 38x SoCs contain two xHCI host. This commit adds the Device Tree description of those interfaces at the SoC level, and also enables the two USB3 ports on the Armada 385 DB platform and one USB3 port on the Armada 385 RD platforms. Signed-off-by: Gregory CLEMENT gregory.clem...@free-electrons.com --- arch/arm/boot/dts/armada-385-db.dts | 8 arch/arm/boot/dts/armada-385-rd.dts | 4 arch/arm/boot/dts/armada-38x.dtsi | 17 + 3 files changed, 29 insertions(+) diff --git a/arch/arm/boot/dts/armada-385-db.dts b/arch/arm/boot/dts/armada-385-db.dts index 6828d77696a6..d5db1466da82 100644 --- a/arch/arm/boot/dts/armada-385-db.dts +++ b/arch/arm/boot/dts/armada-385-db.dts @@ -101,6 +101,14 @@ reg = 0x100 0x3f00; }; }; + + usb3@f { + status = okay; + }; + + usb3@f8000 { + status = okay; + }; }; pcie-controller { diff --git a/arch/arm/boot/dts/armada-385-rd.dts b/arch/arm/boot/dts/armada-385-rd.dts index 45250c88814b..a505fe94ff37 100644 --- a/arch/arm/boot/dts/armada-385-rd.dts +++ b/arch/arm/boot/dts/armada-385-rd.dts @@ -77,6 +77,10 @@ reg = 1; }; }; + + usb3@f { + status = okay; + }; }; pcie-controller { diff --git a/arch/arm/boot/dts/armada-38x.dtsi b/arch/arm/boot/dts/armada-38x.dtsi index a064f59da02d..5913ce1cc601 100644 --- a/arch/arm/boot/dts/armada-38x.dtsi +++ b/arch/arm/boot/dts/armada-38x.dtsi @@ -355,6 +355,23 @@ clocks = coredivclk 0; status = disabled; }; + + usb3@f { + compatible = marvell,armada-380-xhci; + reg = 0xf 0x3fff,0xf4000 0x3fff; s/0x3fff/0x4000/g ? Andrew + interrupts = GIC_SPI 16 IRQ_TYPE_LEVEL_HIGH; + clocks = gateclk 9; + status = disabled; + }; + + usb3@f8000 { + compatible = marvell,armada-380-xhci; + reg = 0xf8000 0x3fff,0xfc000 0x3fff; + interrupts = GIC_SPI 17 IRQ_TYPE_LEVEL_HIGH; + clocks = gateclk 10; + status = disabled; + }; + }; }; -- 1.8.1.2 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 14/18] ARM: mvebu: Add support for USB cluster on the Armada 375 SoC
On Fri, Apr 25, 2014 at 04:07:12PM +0200, Gregory CLEMENT wrote: The Armada 375 SoC comes with an USB2 host and device controller and an USB3 controller. The USB cluster control register allows to manage common features of both USB controllers. Signed-off-by: Gregory CLEMENT gregory.clem...@free-electrons.com --- arch/arm/mach-mvebu/Makefile | 2 +- arch/arm/mach-mvebu/usb-cluster.c | 96 +++ 2 files changed, 97 insertions(+), 1 deletion(-) create mode 100644 arch/arm/mach-mvebu/usb-cluster.c diff --git a/arch/arm/mach-mvebu/Makefile b/arch/arm/mach-mvebu/Makefile index a63e43b6b451..dec05e7e1802 100644 --- a/arch/arm/mach-mvebu/Makefile +++ b/arch/arm/mach-mvebu/Makefile @@ -4,7 +4,7 @@ ccflags-$(CONFIG_ARCH_MULTIPLATFORM) := -I$(srctree)/$(src)/include \ AFLAGS_coherency_ll.o:= -Wa,-march=armv7-a obj-y += system-controller.o mvebu-soc-id.o -obj-$(CONFIG_MACH_MVEBU_V7) += board-v7.o +obj-$(CONFIG_MACH_MVEBU_V7) += board-v7.o usb-cluster.o obj-$(CONFIG_MACH_DOVE) += dove.o obj-$(CONFIG_ARCH_MVEBU) += coherency.o coherency_ll.o pmsu.o obj-$(CONFIG_SMP)+= platsmp.o headsmp.o diff --git a/arch/arm/mach-mvebu/usb-cluster.c b/arch/arm/mach-mvebu/usb-cluster.c new file mode 100644 index ..4c15d282db23 --- /dev/null +++ b/arch/arm/mach-mvebu/usb-cluster.c @@ -0,0 +1,96 @@ +/* + * USB cluster support for Armada 375 platform. + * + * Copyright (C) 2014 Marvell + * + * Gregory CLEMENT gregory.clem...@free-electrons.com + * + * This file is licensed under the terms of the GNU General Public + * License version 2. This program is licensed as is without any + * warranty of any kind, whether express or implied. + * + * Armada 375 comes with an USB2 host and device controller and an + * USB3 controller. The USB cluster control register allows to manage + * common features of both USB controller. + */ + +#include linux/kernel.h +#include linux/init.h +#include linux/of_address.h +#include linux/io.h +#include linux/slab.h + +#define USB2_PHY_CONFIG_ENABLE BIT(0) /* active low */ + +static struct of_device_id of_usb_cluster_table[] = { + { .compatible = marvell,armada-375-usb-cluster, }, + { /* end of list */ }, +}; + +static int __init mvebu_usb_cluster_init(void) +{ + struct device_node *np; + + np = of_find_matching_node(NULL, of_usb_cluster_table); + if (np) { + void __iomem *usb_cluster_base; + u32 reg; + struct device_node *ehci_node, *xhci_node; + struct property *ehci_status; + bool use_usb3 = false; + + usb_cluster_base = of_iomap(np, 0); + BUG_ON(!usb_cluster_base); + + xhci_node = of_find_compatible_node(NULL, NULL, + marvell,armada-375-xhci); + + if (xhci_node of_device_is_available(xhci_node)) + use_usb3 = true; + + ehci_node = of_find_compatible_node(NULL, NULL, + marvell,orion-ehci); + + if (ehci_node of_device_is_available(ehci_node) + use_usb3) { + /* + * We can't use usb2 and usb3 in the same time, so let's + * disbale usb2 and complain about it to the user askinf + * to fix the device tree. + */ + + ehci_status = kzalloc(sizeof(struct property), + GFP_KERNEL); + WARN_ON(!ehci_status); + + ehci_status-value = kstrdup(disabled, GFP_KERNEL); + WARN_ON(!ehci_status-value); + + ehci_status-length = 8; + ehci_status-name = kstrdup(status, GFP_KERNEL); + WARN_ON(!ehci_status-name); + + of_update_property(ehci_node, ehci_status); + pr_err(%s: armada-375-xhci and orion-ehci are incompatible for this SoC.\n, + __func__); + pr_err(Please fix your dts!\n); + pr_err(orion-ehci have been disabled by default...\n); + + } + + reg = readl(usb_cluster_base); + if (use_usb3) + reg |= USB2_PHY_CONFIG_ENABLE; + else + reg = ~USB2_PHY_CONFIG_ENABLE; + writel(reg, usb_cluster_base); Am i right in saying this in the end is about enabling the USB2 phy or a USB3 why? So why not just implement a phy driver? Andrew -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at
Re: [PATCH v8 2/2] usb-serial: Moxa UPORT 12XX/14XX/16XX driver
I hope it's OK that I submit the patch with the below changes to Greg for inclusion in v3.14. Let me know if you prefer to respin yourself. Hi Johan The patch looks good. Acked-by: Andrew Lunn and...@lunn.ch Thanks for all your work with fixing up and mainlining this driver! And thanks for all the reviews. Andrew Happy new year, Johan diff --git a/drivers/usb/serial/mxuport.c b/drivers/usb/serial/mxuport.c index 4b7a5bf57526..17e4f40b6306 100644 --- a/drivers/usb/serial/mxuport.c +++ b/drivers/usb/serial/mxuport.c @@ -229,13 +229,14 @@ static int mxuport_recv_ctrl_urb(struct usb_serial *serial, USB_CTRL_GET_TIMEOUT); if (status 0) { dev_err(serial-interface-dev, - %s - recv usb_control_msg failed. (%d)\n, + %s - usb_control_msg failed (%d)\n, __func__, status); return status; } if (status != size) { - dev_err(serial-interface-dev, %s - sort read (%d / %zd)\n, + dev_err(serial-interface-dev, + %s - short read (%d / %zd)\n, __func__, status, size); return -EIO; } @@ -260,13 +261,14 @@ static int mxuport_send_ctrl_data_urb(struct usb_serial *serial, USB_CTRL_SET_TIMEOUT); if (status 0) { dev_err(serial-interface-dev, - %s - send usb_control_msg failed. (%d)\n, + %s - usb_control_msg failed (%d)\n, __func__, status); return status; } if (status != size) { - dev_err(serial-interface-dev, %s - sort write (%d / %zd)\n, + dev_err(serial-interface-dev, + %s - short write (%d / %zd)\n, __func__, status, size); return -EIO; } @@ -974,8 +976,10 @@ static int mxuport_get_fw_version(struct usb_serial *serial, u32 *version) /* Get firmware version from SDRAM */ err = mxuport_recv_ctrl_urb(serial, RQ_VENDOR_GET_VERSION, 0, 0, ver_buf, 4); - if (err != 4) + if (err != 4) { + err = -EIO; goto out; + } *version = (ver_buf[0] 16) | (ver_buf[1] 8) | ver_buf[2]; err = 0; @@ -1137,7 +1141,10 @@ static int mxuport_port_probe(struct usb_serial_port *port) err = mxuport_send_ctrl_urb(serial, RQ_VENDOR_SET_INTERFACE, MX_INT_RS232, port-port_number); - return err; + if (err) + return err; + + return 0; } static int mxuport_alloc_write_urb(struct usb_serial *serial, @@ -1240,13 +1247,14 @@ static int mxuport_attach(struct usb_serial *serial) /* * All data from the ports is received on the first bulk in * endpoint, with a multiplex header. The second bulk in is - * used for events. Start to read serial data from the device + * used for events. + * + * Start to read from the device. */ err = usb_serial_generic_submit_read_urbs(port0, GFP_KERNEL); if (err) return err; - /* Endpoints on Port1 is used for events */ err = usb_serial_generic_submit_read_urbs(port1, GFP_KERNEL); if (err) { usb_serial_generic_close(port0); @@ -1304,11 +1312,8 @@ static void mxuport_break_ctl(struct tty_struct *tty, int break_state) { struct usb_serial_port *port = tty-driver_data; struct usb_serial *serial = port-serial; - struct mxuport_port *mxport; int enable; - mxport = usb_get_serial_port_data(port); - if (break_state == -1) { enable = 1; dev_dbg(port-dev, %s - sending break\n, __func__); @@ -1384,11 +1389,6 @@ static struct usb_serial_driver *const serial_drivers[] = { module_usb_serial_driver(serial_drivers, mxuport_idtable); -/* - * Module Information - */ MODULE_AUTHOR(Andrew Lunn and...@lunn.ch); MODULE_AUTHOR(supp...@moxa.com); MODULE_LICENSE(GPL); - - -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v8 2/2] usb-serial: Moxa UPORT 12XX/14XX/16XX driver
Add a driver which supports the following Moxa USB to serial converters: * 2 ports : UPort 1250, UPort 1250I * 4 ports : UPort 1410, UPort 1450, UPort 1450I * 8 ports : UPort 1610-8, UPort 1650-8 * 16 ports : UPort 1610-16, UPort 1650-16 The UPORT devices don't directy fit the USB serial model. USB serial assumes a bulk in/out endpoint pair per serial port. Thus a dual port USB serial device is expected to have two bulk in/out pairs. The Moxa UPORT only has one pair for data transfer and places a header on each transfer over the endpoint indicating for which port the transfer relates to. There is a second endpoint pair for events, such as modem control lines changing state, setting baud rates etc. Again, a multiplexing header is used on these endpoints. Some ports need to have a kfifo explicitily allocated since the framework does not allocate one if there is no associated endpoints. The framework will however free it on unload of the module. All data transfers are made on port0, yet the locks are taken on PortN. urb-context points to PortN, even though the URB is for port0. Where possible, code from the generic driver is called. However mxuport_process_read_urb_data() is mostly a cut/paste of usb_serial_generic_process_read_urb(). The driver will attempt to load firmware from userspace and compare the available version and the running version. If the available version is newer, it will be download into RAM of the device and started. This is optional and the driver appears to work O.K. with older firmware in the devices ROM. This driver is based on the MOXA driver and retains MOXAs copyright. Signed-off-by: Andrew Lunn and...@lunn.ch --- v1-v2 Remove debug module parameter. Add missing \n to dev_dbg() strings. Consistent dev_dbg(%s - message in lowercase\n, __func__); Don't log failed allocations. Remove defensive checks for NULL mx_port. Use snprintf() instead of sprintf(). Use port-icount and usb_serial_generic_get_icount(). Break firmware download into smaller functions. Don't DMA to/from buffers on the stack. Use more of the generic write functions. Make use of port_probe() and port_remove(). Indent the device structure. Minor cleanups in mxuport_close() __u8 - u8, __u16 - u16. remove mxuport_wait_until_sent() and implemented mxuport_tx_empty() Remove mxuport_write_room() and use the generic equivelent. Remove unneeded struct mx_uart_config members Make use of generic functions for receiving data and events. Name mxport consistently. Use usb_serial_generic_tiocmiwait() Let line discipline handle TCFLSH ioctl. Import contents of mxuport.h into mxuport.c Use shifts and ORs to create version number. Use port_number, not minor Clarify the meaning of interface in mx_uart_config. Remove buffer from mxuport_send_async_ctrl_urb(). No need to multiply USB_CTRL_SET_TIMEOUT by HZ. Simplify buffer allocation and free. Swap (un)throttle to synchronous interface, remove async code. Pass usb_serial to mxuport_[recv|send]_ctrl_urb() Add missing {} on if else statement. Use unsigned char type, remove casts. Replace constants with defines. Add error handling when disabling HW flow control. Verify port is open before passing it recieved data Verify contents of received data events. Remove broken support for alternative baud rates. Fix B0 and modem line handling. Remove uart_cfg structure. Remove hold_reason, which was set, but never used. s/is/if/ Move parts of port open into port probe. Remove mxport-port and pass usb_serial_port instead. Remove mxport-flags which is no longer used. Allocate buffers before starting firmware download. Remove redundent detected debug message. Use devm_kzalloc() to simply memory handling. Remove repeated debug message when sending break. Implement mxuport_dtr_rts(). Add locking around mcr_state. Remove unused lsr_state from mxuport_port. Support 485 via official IOCTL and sysfs. Use usleep_range() instread of mdelay(). Simplify the comments. Use port0 as a template when setting up bulk out endpoints. Set bulk_in_size for unused endpoints to 0. Rebase to v3.12-rc2 v3-v4 Remove support for rs485 - No hardware to test it on. Fix formatting of multi line comments. Use HEADER_SIZE instead of hard coded 4. Use EIO instread of ECOMM. Drop use of spinlocks on mxuport_{un}throttle(). Remove unneeded parenthesis. Split mxuport_process_read_urb_event() into a number of functions. Check for zero bytes of data. Don't needlessly initialize variables. Fold mxuport_port_init() into mxuport_port_open() Correctly handle the on parameter in mxuport_dtr_rts(). Drop mxuport_get_serial_info and the now empty ioctl handler. Splitup mxuport_set_termios() into smaller functions Make use of C_* macros for cflags. Terminate the firmware download on error. Remove some unneeded dev_dbg()'s. Let usb-serial core fill .write function in usb_serial_driver. Change usb_serial_driver.name to match filename. Move tiocmget() and tiocmset() next to each other. Only one declaration per line
[PATCH v8 1/2] tty: Add C_CMSPAR(tty)
Add the missing C_CMSPAR(tty) macro. Signed-off-by: Andrew Lunn and...@lunn.ch --- include/linux/tty.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/linux/tty.h b/include/linux/tty.h index 97d660ed70c1..e53e90ed3f19 100644 --- a/include/linux/tty.h +++ b/include/linux/tty.h @@ -137,6 +137,7 @@ struct tty_bufhead { #define C_CLOCAL(tty) _C_FLAG((tty), CLOCAL) #define C_CIBAUD(tty) _C_FLAG((tty), CIBAUD) #define C_CRTSCTS(tty) _C_FLAG((tty), CRTSCTS) +#define C_CMSPAR(tty) _C_FLAG((tty), CMSPAR) #define L_ISIG(tty)_L_FLAG((tty), ISIG) #define L_ICANON(tty) _L_FLAG((tty), ICANON) -- 1.8.5.1 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 2/2] usb-serial: Moxa UPORT 12XX/14XX/16XX driver
+* endpoint, with a multiplex header. The second bulk in is +* used for events. Throw away all but the first two bulk in +* urbs. +*/ + for (i = 2; i serial-num_bulk_in; ++i) { + port = serial-port[i]; + for (j = 0; j ARRAY_SIZE(port-read_urbs); ++j) { + usb_free_urb(port-read_urbs[j]); + kfree(port-bulk_in_buffers[j]); + port-read_urbs[j] = NULL; + port-bulk_in_buffers[j] = NULL; + port-bulk_in_size = 0; + } + } Is this really necessary? Why not simple leave these urbs and buffers allocated (if there are any devices with more than to bulk-in endpoints at all)? You never submit them (and neither does usb-serial core since you override resume) so could be left allocated. Hi Johan It is not really necessary. The hardware i have has 4 bulk-in endpoints, of which the driver only uses the first two. I've no idea what the other two are used for. Freeing the buffers does save a little bit of memory, but i could drop this code block and skip the memory saved? Andrew -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 1/2] tty: Add C_CMSPAR(tty)
Add the missing C_CMSPAR(tty) macro. Signed-off-by: Andrew Lunn and...@lunn.ch --- include/linux/tty.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/linux/tty.h b/include/linux/tty.h index 97d660ed70c1..e53e90ed3f19 100644 --- a/include/linux/tty.h +++ b/include/linux/tty.h @@ -137,6 +137,7 @@ struct tty_bufhead { #define C_CLOCAL(tty) _C_FLAG((tty), CLOCAL) #define C_CIBAUD(tty) _C_FLAG((tty), CIBAUD) #define C_CRTSCTS(tty) _C_FLAG((tty), CRTSCTS) +#define C_CMSPAR(tty) _C_FLAG((tty), CMSPAR) #define L_ISIG(tty)_L_FLAG((tty), ISIG) #define L_ICANON(tty) _L_FLAG((tty), ICANON) -- 1.8.5.1 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 2/2] usb-serial: Moxa UPORT 12XX/14XX/16XX driver
Add a driver which supports the following Moxa USB to serial converters: * 2 ports : UPort 1250, UPort 1250I * 4 ports : UPort 1410, UPort 1450, UPort 1450I * 8 ports : UPort 1610-8, UPort 1650-8 * 16 ports : UPort 1610-16, UPort 1650-16 The UPORT devices don't directy fit the USB serial model. USB serial assumes a bulk in/out endpoint pair per serial port. Thus a dual port USB serial device is expected to have two bulk in/out pairs. The Moxa UPORT only has one pair for data transfer and places a header on each transfer over the endpoint indicating for which port the transfer relates to. There is a second endpoint pair for events, such as modem control lines changing state, setting baud rates etc. Again, a multiplexing header is used on these endpoints. Some ports need to have a kfifo explicitily allocated since the framework does not allocate one if there is no associated endpoints. The framework will however free it on unload of the module. All data transfers are made on port0, yet the locks are taken on PortN. urb-context points to PortN, even though the URB is for port0. Where possible, code from the generic driver is called. However mxuport_process_read_urb_data() is mostly a cut/paste of usb_serial_generic_process_read_urb(). The driver will attempt to load firmware from userspace and compare the available version and the running version. If the available version is newer, it will be download into RAM of the device and started. This is optional and the driver appears to work O.K. with older firmware in the devices ROM. This driver is based on the MOXA driver and retains MOXAs copyright. Signed-off-by: Andrew Lunn and...@lunn.ch --- v1-v2 Remove debug module parameter. Add missing \n to dev_dbg() strings. Consistent dev_dbg(%s - message in lowercase\n, __func__); Don't log failed allocations. Remove defensive checks for NULL mx_port. Use snprintf() instead of sprintf(). Use port-icount and usb_serial_generic_get_icount(). Break firmware download into smaller functions. Don't DMA to/from buffers on the stack. Use more of the generic write functions. Make use of port_probe() and port_remove(). Indent the device structure. Minor cleanups in mxuport_close() __u8 - u8, __u16 - u16. remove mxuport_wait_until_sent() and implemented mxuport_tx_empty() Remove mxuport_write_room() and use the generic equivelent. Remove unneeded struct mx_uart_config members Make use of generic functions for receiving data and events. Name mxport consistently. Use usb_serial_generic_tiocmiwait() Let line discipline handle TCFLSH ioctl. Import contents of mxuport.h into mxuport.c Use shifts and ORs to create version number. Use port_number, not minor Clarify the meaning of interface in mx_uart_config. Remove buffer from mxuport_send_async_ctrl_urb(). No need to multiply USB_CTRL_SET_TIMEOUT by HZ. Simplify buffer allocation and free. Swap (un)throttle to synchronous interface, remove async code. Pass usb_serial to mxuport_[recv|send]_ctrl_urb() Add missing {} on if else statement. Use unsigned char type, remove casts. Replace constants with defines. Add error handling when disabling HW flow control. Verify port is open before passing it recieved data Verify contents of received data events. Remove broken support for alternative baud rates. Fix B0 and modem line handling. Remove uart_cfg structure. Remove hold_reason, which was set, but never used. s/is/if/ Move parts of port open into port probe. Remove mxport-port and pass usb_serial_port instead. Remove mxport-flags which is no longer used. Allocate buffers before starting firmware download. Remove redundent detected debug message. Use devm_kzalloc() to simply memory handling. Remove repeated debug message when sending break. Implement mxuport_dtr_rts(). Add locking around mcr_state. Remove unused lsr_state from mxuport_port. Support 485 via official IOCTL and sysfs. Use usleep_range() instread of mdelay(). Simplify the comments. Use port0 as a template when setting up bulk out endpoints. Set bulk_in_size for unused endpoints to 0. Rebase to v3.12-rc2 v3-v4 Remove support for rs485 - No hardware to test it on. Fix formatting of multi line comments. Use HEADER_SIZE instead of hard coded 4. Use EIO instread of ECOMM. Drop use of spinlocks on mxuport_{un}throttle(). Remove unneeded parenthesis. Split mxuport_process_read_urb_event() into a number of functions. Check for zero bytes of data. Don't needlessly initialize variables. Fold mxuport_port_init() into mxuport_port_open() Correctly handle the on parameter in mxuport_dtr_rts(). Drop mxuport_get_serial_info and the now empty ioctl handler. Splitup mxuport_set_termios() into smaller functions Make use of C_* macros for cflags. Terminate the firmware download on error. Remove some unneeded dev_dbg()'s. Let usb-serial core fill .write function in usb_serial_driver. Change usb_serial_driver.name to match filename. Move tiocmget() and tiocmset() next to each other. Only one declaration per line
[PATCH v6 2/2] usb-serial: Moxa UPORT 12XX/14XX/16XX driver
Add a driver which supports the following Moxa USB to serial converters: * 2 ports : UPort 1250, UPort 1250I * 4 ports : UPort 1410, UPort 1450, UPort 1450I * 8 ports : UPort 1610-8, UPort 1650-8 * 16 ports : UPort 1610-16, UPort 1650-16 The UPORT devices don't directy fit the USB serial model. USB serial assumes a bulk in/out endpoint pair per serial port. Thus a dual port USB serial device is expected to have two bulk in/out pairs. The Moxa UPORT only has one pair for data transfer and places a header on each transfer over the endpoint indicating for which port the transfer relates to. There is a second endpoint pair for events, such as modem control lines changing state, setting baud rates etc. Again, a multiplexing header is used on these endpoints. Some ports need to have a kfifo explicitily allocated since the framework does not allocate one if there is no associated endpoints. The framework will however free it on unload of the module. All data transfers are made on port0, yet the locks are taken on PortN. urb-context points to PortN, even though the URB is for port0. Where possible, code from the generic driver is called. However mxuport_process_read_urb_data() is mostly a cut/paste of usb_serial_generic_process_read_urb(). The driver will attempt to load firmware from userspace and compare the available version and the running version. If the available version is newer, it will be download into RAM of the device and started. This is optional and the driver appears to work O.K. with older firmware in the devices ROM. This driver is based on the MOXA driver and retains MOXAs copyright. Signed-off-by: Andrew Lunn and...@lunn.ch --- v1-v2 Remove debug module parameter. Add missing \n to dev_dbg() strings. Consistent dev_dbg(%s - message in lowercase\n, __func__); Don't log failed allocations. Remove defensive checks for NULL mx_port. Use snprintf() instead of sprintf(). Use port-icount and usb_serial_generic_get_icount(). Break firmware download into smaller functions. Don't DMA to/from buffers on the stack. Use more of the generic write functions. Make use of port_probe() and port_remove(). Indent the device structure. Minor cleanups in mxuport_close() __u8 - u8, __u16 - u16. remove mxuport_wait_until_sent() and implemented mxuport_tx_empty() Remove mxuport_write_room() and use the generic equivelent. Remove unneeded struct mx_uart_config members Make use of generic functions for receiving data and events. Name mxport consistently. Use usb_serial_generic_tiocmiwait() Let line discipline handle TCFLSH ioctl. Import contents of mxuport.h into mxuport.c Use shifts and ORs to create version number. Use port_number, not minor Clarify the meaning of interface in mx_uart_config. Remove buffer from mxuport_send_async_ctrl_urb(). No need to multiply USB_CTRL_SET_TIMEOUT by HZ. Simplify buffer allocation and free. Swap (un)throttle to synchronous interface, remove async code. Pass usb_serial to mxuport_[recv|send]_ctrl_urb() Add missing {} on if else statement. Use unsigned char type, remove casts. Replace constants with defines. Add error handling when disabling HW flow control. Verify port is open before passing it recieved data Verify contents of received data events. Remove broken support for alternative baud rates. Fix B0 and modem line handling. Remove uart_cfg structure. Remove hold_reason, which was set, but never used. s/is/if/ Move parts of port open into port probe. Remove mxport-port and pass usb_serial_port instead. Remove mxport-flags which is no longer used. Allocate buffers before starting firmware download. Remove redundent detected debug message. Use devm_kzalloc() to simply memory handling. Remove repeated debug message when sending break. Implement mxuport_dtr_rts(). Add locking around mcr_state. Remove unused lsr_state from mxuport_port. Support 485 via official IOCTL and sysfs. Use usleep_range() instread of mdelay(). Simplify the comments. Use port0 as a template when setting up bulk out endpoints. Set bulk_in_size for unused endpoints to 0. Rebase to v3.12-rc2 v3-v4 Remove support for rs485 - No hardware to test it on. Fix formatting of multi line comments. Use HEADER_SIZE instead of hard coded 4. Use EIO instread of ECOMM. Drop use of spinlocks on mxuport_{un}throttle(). Remove unneeded parenthesis. Split mxuport_process_read_urb_event() into a number of functions. Check for zero bytes of data. Don't needlessly initialize variables. Fold mxuport_port_init() into mxuport_port_open() Correctly handle the on parameter in mxuport_dtr_rts(). Drop mxuport_get_serial_info and the now empty ioctl handler. Splitup mxuport_set_termios() into smaller functions Make use of C_* macros for cflags. Terminate the firmware download on error. Remove some unneeded dev_dbg()'s. Let usb-serial core fill .write function in usb_serial_driver. Change usb_serial_driver.name to match filename. Move tiocmget() and tiocmset() next to each other. Only one declaration per line
[PATCH v6 1/2] tty: Add C_CMSPAR(tty)
Add the missing C_CMSPAR(tty) macro. Signed-off-by: Andrew Lunn and...@lunn.ch --- include/linux/tty.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/linux/tty.h b/include/linux/tty.h index 64f864651d86..8d1ba896070a 100644 --- a/include/linux/tty.h +++ b/include/linux/tty.h @@ -137,6 +137,7 @@ struct tty_bufhead { #define C_CLOCAL(tty) _C_FLAG((tty), CLOCAL) #define C_CIBAUD(tty) _C_FLAG((tty), CIBAUD) #define C_CRTSCTS(tty) _C_FLAG((tty), CRTSCTS) +#define C_CMSPAR(tty) _C_FLAG((tty), CMSPAR) #define L_ISIG(tty)_L_FLAG((tty), ISIG) #define L_ICANON(tty) _L_FLAG((tty), ICANON) -- 1.8.4.rc3 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 2/2] usb-serial: Moxa UPORT 12XX/14XX/16XX driver
Hi Johan Thanks for the quick response. +static int mxuport_dtr(struct usb_serial_port *port, int on) +{ + struct mxuport_port *mxport = usb_get_serial_port_data(port); + int err; + + mutex_lock(mxport-mutex); + + if (on) + mxport-mcr_state |= UART_MCR_DTR; + else + mxport-mcr_state = ~UART_MCR_DTR; + + err = mxuport_set_mcr(port, mxport-mcr_state); + + mutex_unlock(mxport-mutex); + + return err; +} You should probably use SET_DTR (and rename the function mxuport_set_dtr). I considered mxuport_set_dtr(), but it does not really fit with the usb-serial naming. tty_port_operations has dtr_rts, not set_dtr_rts. usb_serial_driver also has dtr_rts, not set_dtr_rts. Andrew -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 1/2] tty: Add C_CMSPAR(tty)
Add the missing C_CMSPAR(tty) macro. Signed-off-by: Andrew Lunn and...@lunn.ch --- include/linux/tty.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/linux/tty.h b/include/linux/tty.h index 64f864651d86..8d1ba896070a 100644 --- a/include/linux/tty.h +++ b/include/linux/tty.h @@ -137,6 +137,7 @@ struct tty_bufhead { #define C_CLOCAL(tty) _C_FLAG((tty), CLOCAL) #define C_CIBAUD(tty) _C_FLAG((tty), CIBAUD) #define C_CRTSCTS(tty) _C_FLAG((tty), CRTSCTS) +#define C_CMSPAR(tty) _C_FLAG((tty), CMSPAR) #define L_ISIG(tty)_L_FLAG((tty), ISIG) #define L_ICANON(tty) _L_FLAG((tty), ICANON) -- 1.8.4.rc3 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 2/2] usb-serial: Moxa UPORT 12XX/14XX/16XX driver
Add a driver which supports the following Moxa USB to serial converters: * 2 ports : UPort 1250, UPort 1250I * 4 ports : UPort 1410, UPort 1450, UPort 1450I * 8 ports : UPort 1610-8, UPort 1650-8 * 16 ports : UPort 1610-16, UPort 1650-16 The UPORT devices don't directy fit the USB serial model. USB serial assumes a bulk in/out endpoint pair per serial port. Thus a dual port USB serial device is expected to have two bulk in/out pairs. The Moxa UPORT only has one pair for data transfer and places a header on each transfer over the endpoint indicating for which port the transfer relates to. There is a second endpoint pair for events, such as modem control lines changing state, setting baud rates etc. Again, a multiplexing header is used on these endpoints. Some ports need to have a kfifo explicitily allocated since the framework does not allocate one if there is no associated endpoints. The framework will however free it on unload of the module. All data transfers are made on port0, yet the locks are taken on PortN. urb-context points to PortN, even though the URB is for port0. Where possible, code from the generic driver is called. However mxuport_process_read_urb_data() is mostly a cut/paste of usb_serial_generic_process_read_urb(). The driver will attempt to load firmware from userspace and compare the available version and the running version. If the available version is newer, it will be download into RAM of the device and started. This is optional and the driver appears to work O.K. with older firmware in the devices ROM. This driver is based on the MOXA driver and retains MOXAs copyright. Signed-off-by: Andrew Lunn and...@lunn.ch --- v1-v2 Remove debug module parameter. Add missing \n to dev_dbg() strings. Consistent dev_dbg(%s - message in lowercase\n, __func__); Don't log failed allocations. Remove defensive checks for NULL mx_port. Use snprintf() instead of sprintf(). Use port-icount and usb_serial_generic_get_icount(). Break firmware download into smaller functions. Don't DMA to/from buffers on the stack. Use more of the generic write functions. Make use of port_probe() and port_remove(). Indent the device structure. Minor cleanups in mxuport_close() __u8 - u8, __u16 - u16. remove mxuport_wait_until_sent() and implemented mxuport_tx_empty() Remove mxuport_write_room() and use the generic equivelent. Remove unneeded struct mx_uart_config members Make use of generic functions for receiving data and events. Name mxport consistently. Use usb_serial_generic_tiocmiwait() Let line discipline handle TCFLSH ioctl. Import contents of mxuport.h into mxuport.c Use shifts and ORs to create version number. Use port_number, not minor Clarify the meaning of interface in mx_uart_config. Remove buffer from mxuport_send_async_ctrl_urb(). No need to multiply USB_CTRL_SET_TIMEOUT by HZ. Simplify buffer allocation and free. Swap (un)throttle to synchronous interface, remove async code. Pass usb_serial to mxuport_[recv|send]_ctrl_urb() Add missing {} on if else statement. Use unsigned char type, remove casts. Replace constants with defines. Add error handling when disabling HW flow control. Verify port is open before passing it recieved data Verify contents of received data events. Remove broken support for alternative baud rates. Fix B0 and modem line handling. Remove uart_cfg structure. Remove hold_reason, which was set, but never used. s/is/if/ Move parts of port open into port probe. Remove mxport-port and pass usb_serial_port instead. Remove mxport-flags which is no longer used. Allocate buffers before starting firmware download. Remove redundent detected debug message. Use devm_kzalloc() to simply memory handling. Remove repeated debug message when sending break. Implement mxuport_dtr_rts(). Add locking around mcr_state. Remove unused lsr_state from mxuport_port. Support 485 via official IOCTL and sysfs. Use usleep_range() instread of mdelay(). Simplify the comments. Use port0 as a template when setting up bulk out endpoints. Set bulk_in_size for unused endpoints to 0. Rebase to v3.12-rc2 v3-v4 Remove support for rs485 - No hardware to test it on. Fix formatting of multi line comments. Use HEADER_SIZE instead of hard coded 4. Use EIO instread of ECOMM. Drop use of spinlocks on mxuport_{un}throttle(). Remove unneeded parenthesis. Split mxuport_process_read_urb_event() into a number of functions. Check for zero bytes of data. Don't needlessly initialize variables. Fold mxuport_port_init() into mxuport_port_open() Correctly handle the on parameter in mxuport_dtr_rts(). Drop mxuport_get_serial_info and the now empty ioctl handler. Splitup mxuport_set_termios() into smaller functions Make use of C_* macros for cflags. Terminate the firmware download on error. Remove some unneeded dev_dbg()'s. Let usb-serial core fill .write function in usb_serial_driver. Change usb_serial_driver.name to match filename. Move tiocmget() and tiocmset() next to each other. Only one declaration per line
Re: [PATCH] usb-serial: Moxa UPORT 12XX/14XX/16XX driver
Hopefully my comments will suffice as guidance for mxuport, but we could iterate the flow-control handling separate from the other changes if you want. Hi Johan I just posted v5. It would be good if you could take a look at the flow-control changes first. Thanks Andrew -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb-serial: Moxa UPORT 12XX/14XX/16XX driver
Hi Johan Thanks for the review. I have a few questions, points to raise. + /* Submit the vendor request */ + buf[0] = data_bits; + buf[1] = parity; + buf[2] = stop_bits; + buf[3] = 0; + + err = mxuport_send_ctrl_data_urb(serial, RQ_VENDOR_SET_LINE, +0, port-port_number, buf, 4); + + if (err != 4) + goto out; + + if (C_BAUD(tty)) { + if (old_termios (old_termios-c_cflag CBAUD) == B0) { + /* Raise DTR/RTS */ + err = mxuport_send_ctrl_urb(serial, RQ_VENDOR_SET_DTR, + 1, port-port_number); + if (err) + goto out; + + mxport-mcr_state |= (UART_MCR_DTR | UART_MCR_RTS); + } else { + /* Drop DTR/RTS */ + err = mxuport_send_ctrl_urb(serial, RQ_VENDOR_SET_DTR, + 0, port-port_number); + if (err) + goto out; + mxport-mcr_state = ~(UART_MCR_DTR | UART_MCR_RTS); + } + err = mxuport_send_ctrl_urb(serial, RQ_VENDOR_SET_MCR, + mxport-mcr_state, + port-port_number); + if (err) + goto out; + } This isn't quite right. You want to raise DTR and (possibly) RTS when transitioning from B0 (or when !old_termios), and drop DTR and RTS when B0 is requested. That is, something like: if (C_BAUD(tty)) { if (old_termios old_termios-c_cflag CBAUD == B0) /* raise DTR/RTS */ } else { /* drop DTR/RTS */ } Which driver is a good example to copy. I looked at a few, and they all seem to do this differently. Maybe something you can put onto you long term TODO list if move most of the logic for this into the generic code, and call the dtr_rts() function of the driver? +static int mxuport_open(struct tty_struct *tty, struct usb_serial_port *port) +{ + struct mxuport_port *mxport = usb_get_serial_port_data(port); + struct usb_serial *serial = port-serial; + int err; + + err = mxuport_send_ctrl_urb(serial, RQ_VENDOR_SET_OPEN, + 1, port-port_number); + if (err) + return err; + + /* Send vendor request - set receive host (enable) */ + err = mxuport_send_ctrl_urb(serial, RQ_VENDOR_SET_RX_HOST_EN, + 1, port-port_number); Do you want to enable reception before opening? + + if (err) { + mxuport_send_ctrl_urb(serial, RQ_VENDOR_SET_OPEN, 0, + port-port_number); + + return err; + } + + /* Initial port termios */ + mxuport_set_termios(tty, port, NULL); + + /* Initial private parameters of port */ + mxport-msr_state = 0; Again, why not use RQ_VENDOR_GET_MSR here? Sorry, i should of put a note in the change log. I looked at this. The problem is, i've no idea what RQ_VENDOR_GET_MSR returns. The Moxa vendor driver never uses it. So setting it to 0 seems safer than wrongly decoding what i get back from the hardware. Andrew -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb-serial: Moxa UPORT 12XX/14XX/16XX driver
Add a driver which supports the following Moxa USB to serial converters: * 2 ports : UPort 1250, UPort 1250I * 4 ports : UPort 1410, UPort 1450, UPort 1450I * 8 ports : UPort 1610-8, UPort 1650-8 * 16 ports : UPort 1610-16, UPort 1650-16 The UPORT devices don't directy fit the USB serial model. USB serial assumes a bulk in/out endpoint pair per serial port. Thus a dual port USB serial device is expected to have two bulk in/out pairs. The Moxa UPORT only has one pair for data transfer and places a header on each transfer over the endpoint indicating for which port the transfer relates to. There is a second endpoint pair for events, just as modem control lines changing state, setting baud rates etc. Again, a multiplexing header is used on these endpoints. Some ports need to have a kfifo explicitily allocated since the framework does not allocate one if there is no associated endpoints. The framework will however free it on unload of the module. All data transfers are made on port0, yet the locks are taken on PortN. urb-context points to PortN, even though the URB is for port0. Where possible, code from the generic driver is called. However mxuport_process_read_urb_data() is mostly a cut/paste of usb_serial_generic_process_read_urb(). The driver will attempt to load firmware from userspace and compare the available version and the running version. If the available version is newer, it will be download into RAM of the device and started. This is optional and the driver appears to work O.K. with older firmware in the devices ROM. This driver is based on the MOXA driver and retains MOXAs copyright. Signed-off-by: Andrew Lunn and...@lunn.ch --- v1-v2 Remove debug module parameter. Add missing \n to dev_dbg() strings. Consistent dev_dbg(%s - message in lowercase\n, __func__); Don't log failed allocations. Remove defensive checks for NULL mx_port. Use snprintf() instead of sprintf(). Use port-icount and usb_serial_generic_get_icount(). Break firmware download into smaller functions. Don't DMA to/from buffers on the stack. Use more of the generic write functions. Make use of port_probe() and port_remove(). Indent the device structure. Minor cleanups in mxuport_close() __u8 - u8, __u16 - u16. remove mxuport_wait_until_sent() and implemented mxuport_tx_empty() Remove mxuport_write_room() and use the generic equivelent. Remove unneeded struct mx_uart_config members Make use of generic functions for receiving data and events. Name mxport consistently. Use usb_serial_generic_tiocmiwait() Let line discipline handle TCFLSH ioctl. Import contents of mxuport.h into mxuport.c Use shifts and ORs to create version number. Use port_number, not minor v2-v3 Clarify the meaning of interface in mx_uart_config. Remove buffer from mxuport_send_async_ctrl_urb(). No need to multiply USB_CTRL_SET_TIMEOUT by HZ. Simplify buffer allocation and free. Swap (un)throttle to synchronous interface, remove async code. Pass usb_serial to mxuport_[recv|send]_ctrl_urb() Add missing {} on if else statement. Use unsigned char type, remove casts. Replace constants with defines. Add error handling when disabling HW flow control. Verify port is open before passing it recieved data Verify contents of received data events. Remove broken support for alternative baud rates. Fix B0 and modem line handling. Remove uart_cfg structure. Remove hold_reason, which was set, but never used. s/is/if/ Move parts of port open into port probe. Remove mxport-port and pass usb_serial_port instead. Remove mxport-flags which is no longer used. Allocate buffers before starting firmware download. Remove redundent detected debug message. Use devm_kzalloc() to simply memory handling. Remove repeated debug message when sending break. Implement mxuport_dtr_rts(). Add locking around mcr_state. Remove unused lsr_state from mxuport_port. Support 485 via official IOCTL and sysfs. Use usleep_range() instread of mdelay(). Simplify the comments. Use port0 as a template when setting up bulk out endpoints. Set bulk_in_size for unused endpoints to 0. Rebase to v3.12-rc2 v3-v4 Remove support for rs485 - No hardware to test it on. Fix formatting of multi line comments. Use HEADER_SIZE instead of hard coded 4. Use EIO instread of ECOMM. Drop use of spinlocks on mxuport_{un}throttle(). Remove unneeded parenthesis. Split mxuport_process_read_urb_event() into a number of functions. Check for zero bytes of data. Don't needlessly initialize variables. Fold mxuport_port_init() into mxuport_port_open() Correctly handle the on parameter in mxuport_dtr_rts(). Drop mxuport_get_serial_info and the now empty ioctl handler. Splitup mxuport_set_termios() into smaller functions Make use of C_* macros for cflags. Terminate the firmware download on error. Remove some unneeded dev_dbg()'s. Let usb-serial core fill .write function in usb_serial_driver. Change usb_serial_driver.name to match filename. Move tiocmget() and tiocmset() next to each other. Only one declaration
Re: [PATCH v3] usb-serial: Moxa UPORT 12XX/14XX/16XX driver
On Tue, Oct 15, 2013 at 08:30:51AM +, Danny Lin (林政易) wrote: Hi Andrew, We can do the interface testing but I want to know how to get the latest source. We need about 5 days to do the testing for all models and give you a testing report. Hi Danny Thanks for the offer of testing. However, I already removed the support from my working tree. How about we leave it out for the moment, get the driver accepted and included in mainline, and then add back the interface support? Andrew -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] usb-serial: Moxa UPORT 12XX/14XX/16XX driver
On Wed, Oct 09, 2013 at 07:27:42PM +0200, Johan Hovold wrote: On Wed, Oct 09, 2013 at 12:57:45PM +0200, Johan Hovold wrote: On Wed, Sep 25, 2013 at 11:53:00AM +0200, Andrew Lunn wrote: [...] +#define MX_INT_RS232 0 +#define MX_INT_2W_RS485 1 +#define MX_INT_RS422 2 +#define MX_INT_4W_RS485 3 +static ssize_t show_four_wire(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct usb_serial_port *port = to_usb_serial_port(dev); + struct mxuport_port *mxport = usb_get_serial_port_data(port); + + return sprintf(buf, %i\n, mxport-four_wire_rs485); Use scnprintf (and PAGE_SIZE) even if it's a bool. +} + +static ssize_t store_four_wire(struct device *dev, +struct device_attribute *attr, +const char *buf, size_t size) +{ + struct usb_serial_port *port = to_usb_serial_port(dev); + struct mxuport_port *mxport = usb_get_serial_port_data(port); + + if (strtobool(buf, mxport-four_wire_rs485) 0) + return -EINVAL; + + return size; +} + +static DEVICE_ATTR(4_wire_rs485, S_IWUSR | S_IRUGO, +show_four_wire, store_four_wire); Use DEVICE_ATTR_RW. You dropped RS422-support from v3. Was that intentional? Hmmm. I've been thinking a bit how best to handle this, and I think we should consider adding a SER_RS485_4_WIRE flag to serial_rs485 instead of having a custom attribute. That still leaves RS422, which could possibly be enabled using the RS485-ioctl as well (or we use a rs422-attribute for now). I'll get back to you on this. After giving this some more thought I don't really think the TIOCSRS485-ioctl interface is appropriate after all. The RQ_VENDOR_SET_INTERFACE-request doesn't just enable 2-wire-rs485-style signalling, but also changes which pins on the DB9-connector that are used. This should probably remain a device-specific parameter. We already have the MOXA mxser-driver with similar configuration options. This one uses a custom ioctl to set the interface, but I think we should use a sysfs parameter for this. What do you say about a simple interface-parameter representing the four MX_INT-values above? A string parameter would perhaps be even better if you combine it with an interface_available-parameter, for example: # cat /sys/bus/usb-serial/devices/ttyUSB0/interface_available rs232 rs422 rs485-2w rs485-4w for devices which can use all four modes. Sorry about the confusion. Hi Johan No problems. I'm actually tempted to drop the support for anything over than RS232. That is all the hardware i have can do. So anything we do add i cannot test. If somebody does have the necessary hardware and the need for these extra modes we can come back to this. Andrew -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3] usb-serial: Moxa UPORT 12XX/14XX/16XX driver
Add a driver which supports the following Moxa USB to serial converters: * 2 ports : UPort 1250, UPort 1250I * 4 ports : UPort 1410, UPort 1450, UPort 1450I * 8 ports : UPort 1610-8, UPort 1650-8 * 16 ports : UPort 1610-16, UPort 1650-16 The UPORT devices don't directy fit the USB serial model. USB serial assumes a bulk in/out endpoint pair per serial port. Thus a dual port USB serial device is expected to have two bulk in/out pairs. The Moxa UPORT only has one pair for data transfer and places a header on each transfer over the endpoint indicating for which port the transfer relates to. There is a second endpoint pair for events, just as modem control lines changing state, setting baud rates etc. Again, a multiplexing header is used on these endpoints. This difference to the model results in some additional code which other drivers don't have: Some ports need to have a kfifo explicitily allocated since the framework does not allocate one if there is no associated endpoints. The framework will however free it on unload of the module. All data transfers are made on port0, yet the locks are taken on PortN. urb-context points to PortN, even though the URB is for port0. Where possible, code from the generic driver is called. However mxuport_process_read_urb_data() is mostly a cut/paste of usb_serial_generic_process_read_urb(). The driver will attempt to load firmware from userspace and compare the available version and the running version. If the available version is newer, it will be download into RAM of the device and started. This is optional and the driver appears to work O.K. with older firmware in the devices ROM. This driver is based on the MOXA driver and retains MOXAs copyright. Signed-off-by: Andrew Lunn and...@lunn.ch --- v1-v2 Remove debug module parameter. Add missing \n to dev_dbg() strings. Consistent dev_dbg(%s - message in lowercase\n, __func__); Don't log failed allocations. Remove defensive checks for NULL mx_port. Use snprintf() instead of sprintf(). Use port-icount and usb_serial_generic_get_icount(). Break firmware download into smaller functions. Don't DMA to/from buffers on the stack. Use more of the generic write functions. Make use of port_probe() and port_remove(). Indent the device structure. Minor cleanups in mxuport_close() __u8 - u8, __u16 - u16. remove mxuport_wait_until_sent() and implemented mxuport_tx_empty() Remove mxuport_write_room() and use the generic equivelent. Remove unneeded struct mx_uart_config members Make use of generic functions for receiving data and events. Name mxport consistently. Use usb_serial_generic_tiocmiwait() Let line discipline handle TCFLSH ioctl. Import contents of mxuport.h into mxuport.c Use shifts and ORs to create version number. Use port_number, not minor v2-v3 Clarify the meaning of interface in mx_uart_config. Remove buffer from mxuport_send_async_ctrl_urb(). No need to multiply USB_CTRL_SET_TIMEOUT by HZ. Simplify buffer allocation and free. Swap (un)throttle to synchronous interface, remove async code. Pass usb_serial to mxuport_[recv|send]_ctrl_urb() Add missing {} on if else statement. Use unsigned char type, remove casts. Replace constants with defines. Add error handling when disabling HW flow control. Verify port is open before passing it recieved data Verify contents of received data events. Remove broken support for alternative baud rates. Fix B0 and modem line handling. Remove uart_cfg structure. Remove hold_reason, which was set, but never used. s/is/if/ Move parts of port open into port probe. Remove mxport-port and pass usb_serial_port instead. Remove mxport-flags which is no longer used. Allocate buffers before starting firmware download. Remove redundent detected debug message. Use devm_kzalloc() to simply memory handling. Remove repeated debug message when sending break. Implement mxuport_dtr_rts(). Add locking around mcr_state. Remove unused lsr_state from mxuport_port. Support 485 via official IOCTL and sysfs. Use usleep_range() instread of mdelay(). Simplify the comments. Use port0 as a template when setting up bulk out endpoints. Set bulk_in_size for unused endpoints to 0. Rebase to v3.12-rc2 TODO Custom resume function --- drivers/usb/serial/Kconfig | 29 + drivers/usb/serial/Makefile |1 + drivers/usb/serial/mxuport.c | 1611 ++ 3 files changed, 1641 insertions(+) create mode 100644 drivers/usb/serial/mxuport.c diff --git a/drivers/usb/serial/Kconfig b/drivers/usb/serial/Kconfig index ddb9c51..36bfd48 100644 --- a/drivers/usb/serial/Kconfig +++ b/drivers/usb/serial/Kconfig @@ -472,6 +472,35 @@ config USB_SERIAL_MOS7840 To compile this driver as a module, choose M here: the module will be called mos7840. If unsure, choose N. +config USB_SERIAL_MOXA_UPORT + tristate USB Moxa UPORT USB Serial Driver + ---help--- + Say Y here if you want to use a MOXA UPort Serial hub
Re: [PATCH v2] usb-serial: Moxa UPORT 12XX/14XX/16XX driver
I need 115200, since that is what most of my development systems use for the serial console. Will the normal path, tty_get_baud_rate(tty), handle baud rates above 57600? Yes, cfsetospeed can be used up to 4Mbaud for example. The hardware is also capable of custom divisor although you don't pass the divisor you need, but the actual baud rate. The moxa vendor driver had a custom IOCTL call for that, which i ripped out, but i've not yet looked how to implement it correctly. Isn't that the way you do it today with RQ_VENDOR_SET_BAUD? Yes, that part i know. I've only used custom divisors once, on an FTDI chipset. As far as i remember, you pass the divisor you want, not the baud rate. I've no idea what the base clock is, before the divider. I've also no idea how the device maps the baud rate to a divisor. So i would need to experiment with different values passed to RQ_VENDOR_SET_BAUD and see what the hardware actually does. For the moment i won't support it. So you are assuming the endpoints all have the same buffer size? At least for the one device i have, that is true. But is it guaranteed? What would happen if the event endpoint actually had a smaller size? No, remember that we're setting up all ports to use the first bulk-out endpoint, so the fields can be retrieved from port0. Doh! I've been away from this code too long. Thanks Andrew -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] usb-serial: Moxa UPORT 12XX/14XX/16XX driver
Hi Johan Thanks for your quick comments. I hopefully have more time to work on this code now, so v3 will not take as long as v2 did. All data transfers are made on port0, yet the locks are taken on PortN. urb-context points to PortN, even though the URL is for port0. You probably meant URB here. Yep. Changelog fixed. +/* This structure holds all of the local port information */ +struct mxuport_port { + int flags; /* for async_struct and serial_struct flags + field */ + int set_B0; + + u8 mcr_state; /* last MCR state */ + + u8 msr_state; /* last MSR state */ + u8 lsr_state; /* last LSR state */ + unsigned long hold_reason; You go to great lengths to update this hold_reason but I can't see that you ever use it? Its left over from the old moxa vendor driver. It seems i ripped out all the users of it. The vendor driver has some flow control between the host and the device. The device has quite big buffers, both read and write. So if the host is writing data out fast, it gets buffered, rather than pushing back and slowing down the application generating the data. Similarly, if the application is being slow reading what has been received, it will get buffered in the device, rather than trigger flow control over the serial link. For my application, using the 4 port device for consoles onto my ARM development systems, these buffers are not an issue. My usage it interactive. However if somebody were to run PPP over the ports, they are likely to see quite bad buffer bloat issues. I will probably rip out the rest of this hold_reason code. If the buffers do turn out to be an issue for somebody, we can see how to cleanly add back the flow control. Its messy because of the multiplexing. + + struct usb_serial_port *port; /* loop back to the owner of this + object */ + struct mx_uart_config *uart_cfg;/* configuration of UART */ +}; + +/* Configuration of UART */ +struct mx_uart_config { + u8 data_bits; /* 5..8 - data bits per character */ + u8 parity; /* parity settings */ + u8 stop_bits; /* stop bits settings */ + u8 flow_ctrl; /* flow control settings*/ + int interface; /* interface is defined */ This description isn't very clear. Agreed. After reading the code, it indicates RS232, 485 etc. I will change the comment. +/* + * mxuport_control_callback + * + * This is the callback function for when we have finished sending + * control urb on the control pipe. + */ +static void mxuport_control_callback(struct urb *urb) +{ + struct usb_ctrlrequest *req = (struct usb_ctrlrequest *)urb-context; You should pass the usb_serial_port (or usb_serial) rather than the ctrlrequest as context in order to get uniform and more informative error messages if anything goes wrong. You can still access the ctrlrequest as urb-setup_packet. + struct device *dev = urb-dev-dev; Use port-dev (or serial-interface-dev). + + switch (urb-status) { + case 0: + break; + + case -ECONNRESET: + case -ENOENT: + case -ESHUTDOWN: + /* This urb is terminated, clean up */ + dev_dbg(dev, %s - urb shutting down with status: %d\n, + __func__, urb-status); + break; + + default: + dev_err(dev, %s - nonzero control status received: %d\n, + __func__, urb-status); + break; + } + + kfree(req); + usb_free_urb(urb); You don't free the transfer_buffer even though you allow control requests to pass a buffer. You don't use that part of the interface at the moment, so either disallow buffers to be passed below or make sure to free them here. I will remove data and size. As you say, the current callers don't actually pass any data. +} + +/* + * mxuport_send_aysnc_ctrl_urb - send asynchronously a vendor request with + * data + * + * This function writes the given buffer out to the control pipe, but + * does not wait around for it to complete. + */ +static void mxuport_send_async_ctrl_urb(struct usb_device *udev, + u8 request, + u16 value, + u16 index, u8 *data, size_t size) ... If you think this could be useful in other contexts than throttle/unthrottle where you currently call it while holding a spinlock, I'd pass the memory flag as a parameter (instead of always using GFP_ATOMIC). At the moment i don't see any other potential users. We can add the memory flag parameter later if needed. + int status = usb_control_msg(udev, +usb_rcvctrlpipe(udev, 0), +
[Pull request] Fix moxa firmware filenames
Hi David, Ben. I messed up with the Moxa firmware. I have the filenames wrong, due to one too many decimal to hex conversions of the USB product ID. I could work around this in the driver, but it would be a lot better to rename the files. Here is a pull request to fix the issue. Thanks Andrew The following changes since commit e7c85b24a5e5107e9911c6b80f536b6bbc5d465d: moxa: Fix firmware file names. (2013-09-05 13:18:07 +0200) are available in the git repository at: g...@github.com:lunn/linux-firmware.git moxa for you to fetch changes up to e7c85b24a5e5107e9911c6b80f536b6bbc5d465d: moxa: Fix firmware file names. (2013-09-05 13:18:07 +0200) -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PULL] Moxa UPort firmware
On Thu, Aug 08, 2013 at 11:00:20PM +0200, Ben Hutchings wrote: On Thu, 2013-08-08 at 12:20 +0200, Andrew Lunn wrote: Hi Ben, David Here is a pull request for firmware for Moxa USB-Serial hub devices. Thanks Andrew The following changes since commit 931e4469dc254df66a2c990ff1a8723685759eb4: radeon: add ucode for KABINI GPUs (2013-07-28 22:39:47 +0100) are available in the git repository at: https://github.com/lunn/linux-firmware.git moxa for you to fetch changes up to 6eed67a9f372a5ade30431af9af0aff8df6f678f: moxa: Add firmware for some of the Moxa USB-Serial hubs (2013-08-08 12:04:52 +0200) [...] Pulled and pushed out, thanks. Great thanks. Is it too late to add the Acked-by from Moxa? It would help make it clear that Moxa fully agree and helped. Thanks Andrew -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PULL] Moxa UPort firmware
Hi Ben, David Here is a pull request for firmware for Moxa USB-Serial hub devices. Thanks Andrew The following changes since commit 931e4469dc254df66a2c990ff1a8723685759eb4: radeon: add ucode for KABINI GPUs (2013-07-28 22:39:47 +0100) are available in the git repository at: https://github.com/lunn/linux-firmware.git moxa for you to fetch changes up to 6eed67a9f372a5ade30431af9af0aff8df6f678f: moxa: Add firmware for some of the Moxa USB-Serial hubs (2013-08-08 12:04:52 +0200) Andrew Lunn (1): moxa: Add firmware for some of the Moxa USB-Serial hubs WHENCE| 32 moxa/moxa-04e2.fw | Bin 0 - 33681 bytes moxa/moxa-04e3.fw | Bin 0 - 33685 bytes moxa/moxa-0582.fw | Bin 0 - 33521 bytes moxa/moxa-05aa.fw | Bin 0 - 33521 bytes moxa/moxa-05ab.fw | Bin 0 - 33525 bytes moxa/moxa-064d.fw | Bin 0 - 33529 bytes moxa/moxa-0652.fw | Bin 0 - 33525 bytes moxa/moxa-0675.fw | Bin 0 - 33529 bytes moxa/moxa-067a.fw | Bin 0 - 33525 bytes 10 files changed, 32 insertions(+) create mode 100644 moxa/moxa-04e2.fw create mode 100644 moxa/moxa-04e3.fw create mode 100644 moxa/moxa-0582.fw create mode 100644 moxa/moxa-05aa.fw create mode 100644 moxa/moxa-05ab.fw create mode 100644 moxa/moxa-064d.fw create mode 100644 moxa/moxa-0652.fw create mode 100644 moxa/moxa-0675.fw create mode 100644 moxa/moxa-067a.fw -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] usb-serial: Moxa UPORT 12XX/14XX/16XX driver
On Sun, May 26, 2013 at 06:04:24PM +0200, Johan Hovold wrote: On Sun, May 26, 2013 at 01:00:51PM +0200, Andrew Lunn wrote: +/* + * mxuport_write_room + * + * Return how much space is available in the buffer. + */ +static int mxuport_write_room(struct tty_struct *tty) +{ + struct usb_serial_port *port = tty-driver_data; + unsigned long flags; + int room; + + spin_lock_irqsave(port-lock, flags); + room = kfifo_avail(port-write_fifo); + spin_unlock_irqrestore(port-lock, flags); + + dev_dbg(port-dev, %s - returns %d\n, __func__, room); + return room; +} It seems it could be possible to reuse the generic write implementations of all functions but prepare_write_buffer above rather than the modified copies. Hi Johan If there a reason why usb_serial_generic_write_room() is not exported as a GPL symbol. It looks like i should be able to use it once i sort my 'fake' ports/endpoints out. Thanks Andrew -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] usb-serial: Moxa UPORT 12XX/14XX/16XX driver
On Tue, May 28, 2013 at 11:08:54PM +0200, Johan Hovold wrote: On Tue, May 28, 2013 at 09:41:08PM +0200, Andrew Lunn wrote: On Sun, May 26, 2013 at 06:04:24PM +0200, Johan Hovold wrote: On Sun, May 26, 2013 at 01:00:51PM +0200, Andrew Lunn wrote: +/* + * mxuport_write_room + * + * Return how much space is available in the buffer. + */ +static int mxuport_write_room(struct tty_struct *tty) +{ + struct usb_serial_port *port = tty-driver_data; + unsigned long flags; + int room; + + spin_lock_irqsave(port-lock, flags); + room = kfifo_avail(port-write_fifo); + spin_unlock_irqrestore(port-lock, flags); + + dev_dbg(port-dev, %s - returns %d\n, __func__, room); + return room; +} It seems it could be possible to reuse the generic write implementations of all functions but prepare_write_buffer above rather than the modified copies. Hi Johan If there a reason why usb_serial_generic_write_room() is not exported as a GPL symbol. It looks like i should be able to use it once i sort my 'fake' ports/endpoints out. No, I don't think so, but perhaps it should be. Most usb-serial drivers still use it. Simply leave the operation unset and usb-serial core will set it for you. Hi Johan Ah, i fell into the trap. I grep'd for usb_serial_generic_write_room and did not get any hits. The code is obfusticated/grep unfriendly: set_to_generic_if_null(device, write_room); I will leave it unset. Andrew -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/2] MOXA UPORT Serial USB driver
These two patches add support for MOXA 12XX/14XX/16XX USB serial devices. The first patch exports a function from the generic usb serial driver, which the driver in the second patch would like to use. Andrew Lunn (2): USB: Serial: export usb_serial_generic_submit_read_urb() usb-serial: Moxa UPORT 12XX/14XX/16XX driver drivers/usb/serial/Kconfig | 30 + drivers/usb/serial/Makefile |1 + drivers/usb/serial/generic.c |5 +- drivers/usb/serial/mxuport.c | 2005 ++ drivers/usb/serial/mxuport.h | 184 include/linux/usb/serial.h |2 + 6 files changed, 2225 insertions(+), 2 deletions(-) create mode 100644 drivers/usb/serial/mxuport.c create mode 100644 drivers/usb/serial/mxuport.h -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] USB: Serial: export usb_serial_generic_submit_read_urb()
USB Serial drivers can currently use the generic function usb_serial_generic_submit_read_urbs() to submit a group of urbs when starting up the device. The read bulk callback needs to be able to submit a single urb. usb_serial_generic_submit_read_urb() does this, but it is not exported and so only usable within generic functions, in particular usb_serial_generic_read_bulk_callback(). Export this function so drivers can use it in there own read bulk callback routine. Signed-off-by: Andrew Lunn and...@lunn.ch --- drivers/usb/serial/generic.c |5 +++-- include/linux/usb/serial.h |2 ++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/usb/serial/generic.c b/drivers/usb/serial/generic.c index 297665f..13e9b3b 100644 --- a/drivers/usb/serial/generic.c +++ b/drivers/usb/serial/generic.c @@ -253,8 +253,8 @@ int usb_serial_generic_chars_in_buffer(struct tty_struct *tty) } EXPORT_SYMBOL_GPL(usb_serial_generic_chars_in_buffer); -static int usb_serial_generic_submit_read_urb(struct usb_serial_port *port, - int index, gfp_t mem_flags) +int usb_serial_generic_submit_read_urb(struct usb_serial_port *port, + int index, gfp_t mem_flags) { int res; @@ -276,6 +276,7 @@ static int usb_serial_generic_submit_read_urb(struct usb_serial_port *port, return 0; } +EXPORT_SYMBOL_GPL(usb_serial_generic_submit_read_urb); int usb_serial_generic_submit_read_urbs(struct usb_serial_port *port, gfp_t mem_flags) diff --git a/include/linux/usb/serial.h b/include/linux/usb/serial.h index b9b0f7b4..a85e35b 100644 --- a/include/linux/usb/serial.h +++ b/include/linux/usb/serial.h @@ -337,6 +337,8 @@ extern int usb_serial_generic_get_icount(struct tty_struct *tty, struct serial_icounter_struct *icount); extern int usb_serial_generic_register(void); extern void usb_serial_generic_deregister(void); +extern int usb_serial_generic_submit_read_urb(struct usb_serial_port *port, + int index, gfp_t mem_flags); extern int usb_serial_generic_submit_read_urbs(struct usb_serial_port *port, gfp_t mem_flags); extern void usb_serial_generic_process_read_urb(struct urb *urb); -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] usb-serial: Moxa UPORT 12XX/14XX/16XX driver
On Sun, May 26, 2013 at 06:04:24PM +0200, Johan Hovold wrote: On Sun, May 26, 2013 at 01:00:51PM +0200, Andrew Lunn wrote: Add a driver which supports the following Moxa USB to serial converters: * 2 ports : UPort 1250, UPort 1250I * 4 ports : UPort 1410, UPort 1450, UPort 1450I * 8 ports : UPort 1610-8, UPort 1650-8 * 16 ports : UPort 1610-16, UPort 1650-16 The UPORT devices don't directy fit the USB serial model. USB serial assumes a bulk in/out endpoint pair per serial port. Thus a dual port USB serial device is expected to have two bulk in/out pairs. The Moxa UPORT only has one pair for data transfer and places a header on each transfer over the endpoint indicating for which port the transfer relates to. There is a second endpoint pair for events, just as modem control lines changing state, setting baud rates etc. Again, a multiplexing header is used on these endpoints. This difference to the model results in some additional code which other drivers don't have: Some ports need to have a kfifo explicitily allocated since the framework does not allocate one if there is no associated endpoints. The framework will however free it on unload of the module. All data transfers are made on port0, yet the locks are taken on PortN. urb-context points to PortN, even though the URL is for port0. First, thanks for submitting the driver. And thanks for the quick review. As you mention, it does not fit the usb-serial model directly, but it seems that there's still quite a bit more of generic code that could be reused rather than copied or reimplemented. I will take another look. I was not happy with cut/pasting code from the generic driver, so tried to minimize it. I will take a deeper look at your suggestions with the endpoints. Consider my comments as a first round of review, there's probably some things I've missed and more minor things I'll get back to later. Sure, no problem. The driver will attempt to load firmware from userspace and compare the available version and the running version. If the available version is newer, it will be download into RAM of the device and started. This is optional and currently the firmware blobs are not yet in linux-firmware. However MOXA have agreed that the blobs can be distributed. This driver is based on the MOXA driver and retains MOXAs copyright. The driver is also quite heavily based on the generic driver and much code is copied more or less verbatim. Please make sure to mention in this in the header as well. O.K. Also, most of the paranoid checks for NULL pointers and closed ports is from the original MOXA driver. Maybe in the past they have had problems. I've already take out a lot of dead code, and converted to generic where i could. I'm happy to continue this. +/* Global variables */ +static bool debug; Please drop the debug module parameter. usb_serial_debug_data is supposed to be controlled using dynamic debugging. O.K. + +/* Table of devices that work with this driver */ +static struct usb_device_id mxuport_idtable[] = { + {USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1250_PID)}, + {USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1251_PID)}, + {USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1410_PID)}, + {USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1450_PID)}, + {USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1451_PID)}, + {USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1618_PID)}, + {USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1658_PID)}, + {USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1613_PID)}, + {USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1653_PID)}, + {} /* Terminating entry */ +}; + +MODULE_DEVICE_TABLE(usb, mxuport_idtable); + +/* + * mxuport_prepare_write_buffer - fill in the buffer, ready for + * sending + * + * Add a four byte header containing the port number and the number of + * bytes of data in the message. Return the number of bytes in the + * buffer. + */ +static int mxuport_prepare_write_buffer(struct usb_serial_port *port, + void *dest, size_t size) +{ + struct mxuport_port *mx_port = usb_get_serial_port_data(port); + unsigned char *buf = dest; + int count; + + count = kfifo_out_locked(port-write_fifo, buf + 4, size - 4, +port-lock); + + put_unaligned_be16(mx_port-portno, buf); + put_unaligned_be16(count, buf + 2); + + dev_dbg(port-dev, %s - port %d, size %d count %d, __func__, + mx_port-portno, size, count); + + return count + 4; +} + +/* + * mxuport_write_start - kick off an URB write + * @port: Pointer to the struct usb_serial_port data + * + * Returns zero on success, or a negative errno value + */ +static int mxuport_write_start(struct usb_serial_port *port) +{ + struct usb_serial *serial = port-serial; + struct urb *urb; + int count, result
Re: [PATCH 2/2] USB: EHCI: make ehci-orion a separate driver
On Mon, Feb 18, 2013 at 05:34:35PM -0500, Alan Stern wrote: On Fri, 15 Feb 2013, Arnd Bergmann wrote: From: Manjunath Goudar manjunath.gou...@linaro.org With the multiplatform changes in arm-soc tree, it becomes possible to enable the mvebu platform (which uses ehci-orion) at the same time as other platforms that require a conflicting EHCI bus glue. At the moment, this results in a warning like drivers/usb/host/ehci-hcd.c:1297:0: warning: PLATFORM_DRIVER redefined [enabled by default] drivers/usb/host/ehci-hcd.c:1277:0: note: this is the location of the previous definition drivers/usb/host/ehci-orion.c:334:31: warning: 'ehci_orion_driver' defined but not used [-Wunused-variable] and an ehci driver that only works on one of them. With the infrastructure added by Alan Stern in patch 3e0232039 USB: EHCI: prepare to make ehci-hcd a library module, we can avoid this problem by turning a bus glue into a separate module, as we do here for the orion bus glue. --- a/drivers/usb/host/Makefile +++ b/drivers/usb/host/Makefile @@ -30,6 +30,7 @@ obj-$(CONFIG_USB_EHCI_MXC)+= ehci-mxc.o obj-$(CONFIG_USB_OXU210HP_HCD) += oxu210hp-hcd.o obj-$(CONFIG_USB_EHCI_HCD_VT8500)+= ehci-vt8500.o +obj-$(CONFIG_USB_EHCI_HCD_ORION)+= ehci-orion.o Both of these two new lines should be formatted like the other lines in this file (i.e., with tabs at the corresponding places), and they should come before the OXU210HP_HCD entry so that they are next to the other EHCI-related lines. --- a/drivers/usb/host/ehci-orion.c +++ b/drivers/usb/host/ehci-orion.c @@ -17,6 +17,13 @@ #include linux/of.h #include linux/of_device.h #include linux/of_irq.h +#include linux/usb.h +#include linux/usb/hcd.h +#include linux/io.h +#include linux/dma-mapping.h Is this line really needed? @@ -34,6 +41,17 @@ #define USB_PHY_IVREF_CTRL 0x440 #define USB_PHY_TST_GRP_CTRL 0x450 +#define DRIVER_DESC EHCI orion driver + +static const char hcd_name[] = ehci-orion; + +static struct hc_driver __read_mostly ehci_orion_hc_driver; + +static const struct ehci_driver_overrides orion_overrides __initdata = { + .reset = ehci_setup, +}; This is not necessary; ehci_setup is the default value anyway. This structure can be omitted. @@ -323,8 +296,6 @@ static int __exit ehci_orion_drv_remove(struct platform_device *pdev) return 0; } -MODULE_ALIAS(platform:orion-ehci); - static const struct of_device_id ehci_orion_dt_ids[] = { { .compatible = marvell,orion-ehci, }, {}, @@ -336,8 +307,31 @@ static struct platform_driver ehci_orion_driver = { .remove = __exit_p(ehci_orion_drv_remove), .shutdown = usb_hcd_platform_shutdown, .driver = { - .name = orion-ehci, + .name = hcd_name, Is this really what you want -- changing the driver name from orion-ehci to ehci-orion? Is that liable to cause trouble? +MODULE_DESCRIPTION(DRIVER_DESC); +MODULE_ALIAS(platform:ehci-orion); And is this really what you want -- changing the alias from platform:orion-ehci to platform:ehci-orion? Hi Manjunath I can confirm that this breaks non DT based kirkwood systems. The driver does not get loaded. Sorry for not testing and finding this case earlier, i just tested a DT based system. GregKH: Please can you drop this patch from usb-next. It breaks more than it fixes. Andrew -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] USB: EHCI: make ehci-orion a separate driver
On Fri, Feb 15, 2013 at 11:12:29PM +0100, Arnd Bergmann wrote: From: Manjunath Goudar manjunath.gou...@linaro.org With the multiplatform changes in arm-soc tree, it becomes possible to enable the mvebu platform (which uses ehci-orion) at the same time as other platforms that require a conflicting EHCI bus glue. At the moment, this results in a warning like drivers/usb/host/ehci-hcd.c:1297:0: warning: PLATFORM_DRIVER redefined [enabled by default] drivers/usb/host/ehci-hcd.c:1277:0: note: this is the location of the previous definition drivers/usb/host/ehci-orion.c:334:31: warning: 'ehci_orion_driver' defined but not used [-Wunused-variable] and an ehci driver that only works on one of them. With the infrastructure added by Alan Stern in patch 3e0232039 USB: EHCI: prepare to make ehci-hcd a library module, we can avoid this problem by turning a bus glue into a separate module, as we do here for the orion bus glue. Signed-off-by: Manjunath Goudar manjunath.gou...@linaro.org Signed-off-by: Arnd Bergmann a...@arndb.de Cc: Jason Cooper ja...@lakedaemon.net Cc: Andrew Lunn and...@lunn.ch --- drivers/usb/host/Kconfig | 8 drivers/usb/host/Makefile | 1 + drivers/usb/host/ehci-hcd.c | 6 +-- drivers/usb/host/ehci-orion.c | 90 --- 4 files changed, 52 insertions(+), 53 deletions(-) Tested-by: Andrew Lunn and...@lunn.ch I booted with this patch on a Kirkwood based QNAP NAS box. The USB bus enumerated as expected. Andrew -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/2] ARM: Kirkwood: Convert all DT boards to EHCI via DT.
Now that the EHCI driver has DT support, drop old style configuration of it and add DT in its place. Since all the boards enable the EHCI, enable it by default in kirkwood.dtsi. Any new boards which don't have USB can specifically disable it. Signed-off-by: Andrew Lunn and...@lunn.ch --- arch/arm/boot/dts/kirkwood.dtsi |7 +++ arch/arm/mach-kirkwood/board-dnskw.c |1 - arch/arm/mach-kirkwood/board-dockstar.c |1 - arch/arm/mach-kirkwood/board-dreamplug.c |1 - arch/arm/mach-kirkwood/board-dt.c |1 + arch/arm/mach-kirkwood/board-goflexnet.c |1 - arch/arm/mach-kirkwood/board-ib62x0.c |1 - arch/arm/mach-kirkwood/board-iconnect.c |1 - arch/arm/mach-kirkwood/board-iomega_ix2_200.c |2 -- arch/arm/mach-kirkwood/board-km_kirkwood.c|1 - arch/arm/mach-kirkwood/board-lsxl.c |1 - arch/arm/mach-kirkwood/board-ts219.c |1 - 12 files changed, 8 insertions(+), 11 deletions(-) diff --git a/arch/arm/boot/dts/kirkwood.dtsi b/arch/arm/boot/dts/kirkwood.dtsi index 4e5b815..4fc7a8e 100644 --- a/arch/arm/boot/dts/kirkwood.dtsi +++ b/arch/arm/boot/dts/kirkwood.dtsi @@ -77,6 +77,13 @@ status = okay; }; + ehci@5 { + compatible = marvell,orion-ehci; + reg = 0x5 0x1000; + interrupts = 19; + status = okay; + }; + sata@8 { compatible = marvell,orion-sata; reg = 0x8 0x5000; diff --git a/arch/arm/mach-kirkwood/board-dnskw.c b/arch/arm/mach-kirkwood/board-dnskw.c index 43d16d6..2ac6c60 100644 --- a/arch/arm/mach-kirkwood/board-dnskw.c +++ b/arch/arm/mach-kirkwood/board-dnskw.c @@ -78,7 +78,6 @@ void __init dnskw_init(void) { kirkwood_mpp_conf(dnskw_mpp_config); - kirkwood_ehci_init(); kirkwood_ge00_init(dnskw_ge00_data); /* Register power-off GPIO. */ diff --git a/arch/arm/mach-kirkwood/board-dockstar.c b/arch/arm/mach-kirkwood/board-dockstar.c index f2fbb02..e94782d 100644 --- a/arch/arm/mach-kirkwood/board-dockstar.c +++ b/arch/arm/mach-kirkwood/board-dockstar.c @@ -55,7 +55,6 @@ void __init dockstar_dt_init(void) if (gpio_request(29, USB Power Enable) != 0 || gpio_direction_output(29, 1) != 0) pr_err(can't setup GPIO 29 (USB Power Enable)\n); - kirkwood_ehci_init(); kirkwood_ge00_init(dockstar_ge00_data); } diff --git a/arch/arm/mach-kirkwood/board-dreamplug.c b/arch/arm/mach-kirkwood/board-dreamplug.c index 20af53a..acdc04a 100644 --- a/arch/arm/mach-kirkwood/board-dreamplug.c +++ b/arch/arm/mach-kirkwood/board-dreamplug.c @@ -64,7 +64,6 @@ void __init dreamplug_init(void) */ kirkwood_mpp_conf(dreamplug_mpp_config); - kirkwood_ehci_init(); kirkwood_ge00_init(dreamplug_ge00_data); kirkwood_ge01_init(dreamplug_ge01_data); kirkwood_sdio_init(dreamplug_mvsdio_data); diff --git a/arch/arm/mach-kirkwood/board-dt.c b/arch/arm/mach-kirkwood/board-dt.c index 70c5a28..ccb91b8 100644 --- a/arch/arm/mach-kirkwood/board-dt.c +++ b/arch/arm/mach-kirkwood/board-dt.c @@ -34,6 +34,7 @@ struct of_dev_auxdata kirkwood_auxdata_lookup[] __initdata = { OF_DEV_AUXDATA(marvell,orion-sata, 0xf108, sata_mv.0, NULL), OF_DEV_AUXDATA(marvell,orion-nand, 0xf400, orion_nand, NULL), OF_DEV_AUXDATA(marvell,orion-crypto, 0xf103, mv_crypto, NULL), + OF_DEV_AUXDATA(marvell,orion-ehci, 0xf105, orion-ehci.0, NULL), {}, }; diff --git a/arch/arm/mach-kirkwood/board-goflexnet.c b/arch/arm/mach-kirkwood/board-goflexnet.c index 001ca8c..d388bea 100644 --- a/arch/arm/mach-kirkwood/board-goflexnet.c +++ b/arch/arm/mach-kirkwood/board-goflexnet.c @@ -65,7 +65,6 @@ void __init goflexnet_init(void) if (gpio_request(29, USB Power Enable) != 0 || gpio_direction_output(29, 1) != 0) pr_err(can't setup GPIO 29 (USB Power Enable)\n); - kirkwood_ehci_init(); kirkwood_ge00_init(goflexnet_ge00_data); } diff --git a/arch/arm/mach-kirkwood/board-ib62x0.c b/arch/arm/mach-kirkwood/board-ib62x0.c index cfc47f8..db08e37 100644 --- a/arch/arm/mach-kirkwood/board-ib62x0.c +++ b/arch/arm/mach-kirkwood/board-ib62x0.c @@ -61,7 +61,6 @@ void __init ib62x0_init(void) */ kirkwood_mpp_conf(ib62x0_mpp_config); - kirkwood_ehci_init(); kirkwood_ge00_init(ib62x0_ge00_data); if (gpio_request(IB62X0_GPIO_POWER_OFF, ib62x0:power:off) == 0 gpio_direction_output(IB62X0_GPIO_POWER_OFF, 0) == 0) diff --git a/arch/arm/mach-kirkwood/board-iconnect.c b/arch/arm/mach-kirkwood/board-iconnect.c index d084b1e..8275fb0 100644 --- a/arch/arm/mach-kirkwood/board-iconnect.c +++ b/arch/arm/mach-kirkwood/board-iconnect.c @@ -45,7 +45,6 @@ void
Re: [PATCH 1/2] ARM: Kirkwood: ehci-orion: Add device tree binding
+Required properties: +- compatible: must be marvell,orion-ehci +- reg: physical base address of the controller and length of memory mapped + region. +- interrupts: The EHCI interrupt +- phy-version: Can be one of: + NA - Don't touch the phy, something else has already configured it. + orion5x - PHY setup as specified by the Orion5x Errata + +Example: + + ehci@5 { + compatible = marvell,orion-ehci; + reg = 0x5 0x1000; + interrupts = 19; + phy-version = NA; + }; This isn't an appropriate binding for phy. I know, it maps straight over from the platform data, but it doesn't focus on what the actual hardware is. A couple of options. What probably makes most sense depending on how other phy bindings are moving ahead is to add a phy node under the ehci controller for the orion5x case, and have an appropriate compatible value there. No node means the same as NA in the above binding. Alternatively, have a phy phandle that points to the phy device if it sits on an i2c bus, etc. I Olaf Could i suggest a third option: I just drop USB phy configuration all together. Only mach-orion5x needs this and nobody has shown any interest in moving mach-orion5x to DT. So i would just hard code it to NA. If anybody does show interest in DT for orion5x, we can add a phy node under ehci as a pure extension which does not affect backward compatibility. Thanks Andrew -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] USB: PLAT_ORION fulfils USB_ARCH_HAS_EHCI
The various Orion SoCs, i.e. orion5x, kirkwood, dove, mv78xx0 and 370/XP have EHCI. Make sure USB_ARCH_HAS_EHCI reflects this. Reported-by: Sebastian Hesselbarth sebastian.hesselba...@gmail.com Signed-off-by: Andrew Lunn and...@lunn.ch --- drivers/usb/Kconfig |1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig index a7773a3..723efcc 100644 --- a/drivers/usb/Kconfig +++ b/drivers/usb/Kconfig @@ -48,6 +48,7 @@ config USB_ARCH_HAS_EHCI default y if SPARC_LEON default y if ARCH_MMP default y if MACH_LOONGSON1 + default y if PLAT_ORION default PCI # some non-PCI HCDs implement xHCI -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] ARM: Kirkwood: ehci-orion: Add device tree binding
Based on previous work by Michael Walle and Jason Cooper. Made their work actually work, which required added interrupt from DT and auxdata, along with setting the dma_mask, which DT does not currently do. Signed-off-by: Andrew Lunn and...@lunn.ch --- .../devicetree/bindings/usb/ehci-orion.txt | 19 +++ drivers/usb/host/ehci-orion.c | 59 +++- 2 files changed, 75 insertions(+), 3 deletions(-) create mode 100644 Documentation/devicetree/bindings/usb/ehci-orion.txt diff --git a/Documentation/devicetree/bindings/usb/ehci-orion.txt b/Documentation/devicetree/bindings/usb/ehci-orion.txt new file mode 100644 index 000..1bd704e --- /dev/null +++ b/Documentation/devicetree/bindings/usb/ehci-orion.txt @@ -0,0 +1,19 @@ +* EHCI controller, Orion Marvell variants + +Required properties: +- compatible: must be marvell,orion-ehci +- reg: physical base address of the controller and length of memory mapped + region. +- interrupts: The EHCI interrupt +- phy-version: Can be one of: + NA - Don't touch the phy, something else has already configured it. + orion5x - PHY setup as specified by the Orion5x Errata + +Example: + + ehci@5 { + compatible = marvell,orion-ehci; + reg = 0x5 0x1000; + interrupts = 19; + phy-version = NA; + }; diff --git a/drivers/usb/host/ehci-orion.c b/drivers/usb/host/ehci-orion.c index b34b928..605feb09 100644 --- a/drivers/usb/host/ehci-orion.c +++ b/drivers/usb/host/ehci-orion.c @@ -14,6 +14,9 @@ #include linux/mbus.h #include linux/clk.h #include linux/platform_data/ehci-orion.h +#include linux/of.h +#include linux/of_device.h +#include linux/of_irq.h #define rdl(off) __raw_readl(hcd-regs + (off)) #define wrl(off, val) __raw_writel((val), hcd-regs + (off)) @@ -181,6 +184,27 @@ ehci_orion_conf_mbus_windows(struct usb_hcd *hcd, } } +static const int get_phy_version(struct device_node *np) +{ + const char *pm; + int err; + + err = of_property_read_string(np, phy-version, pm); + + if (err 0) + return err; + + + if (!strcasecmp(pm, NA)) + return EHCI_PHY_NA; + if (!strcasecmp(pm, orion5x)) + return EHCI_PHY_ORION; + + return -ENODEV; +} + +static u64 ehci_orion_dma_mask = DMA_BIT_MASK(32); + static int __devinit ehci_orion_drv_probe(struct platform_device *pdev) { struct orion_ehci_data *pd = pdev-dev.platform_data; @@ -191,13 +215,17 @@ static int __devinit ehci_orion_drv_probe(struct platform_device *pdev) struct clk *clk; void __iomem *regs; int irq, err; + enum orion_ehci_phy_ver phy_version; if (usb_disabled()) return -ENODEV; pr_debug(Initializing Orion-SoC USB Host Controller\n); - irq = platform_get_irq(pdev, 0); + if (pdev-dev.of_node) + irq = irq_of_parse_and_map(pdev-dev.of_node, 0); + else + irq = platform_get_irq(pdev, 0); if (irq = 0) { dev_err(pdev-dev, Found HC with no IRQ. Check %s setup!\n, @@ -215,6 +243,14 @@ static int __devinit ehci_orion_drv_probe(struct platform_device *pdev) goto err1; } + /* +* Right now device-tree probed devices don't get dma_mask +* set. Since shared usb code relies on it, set it here for +* now. Once we have dma capability bindings this can go away. +*/ + if (!pdev-dev.dma_mask) + pdev-dev.dma_mask = ehci_orion_dma_mask; + if (!request_mem_region(res-start, resource_size(res), ehci_orion_hc_driver.description)) { dev_dbg(pdev-dev, controller already in use\n); @@ -262,7 +298,14 @@ static int __devinit ehci_orion_drv_probe(struct platform_device *pdev) /* * setup Orion USB controller. */ - switch (pd-phy_version) { + if (pdev-dev.of_node) { + phy_version = get_phy_version(pdev-dev.of_node); + if (phy_version 0) + goto err3; + } else + phy_version = pd-phy_version; + + switch (phy_version) { case EHCI_PHY_NA: /* dont change USB phy settings */ break; case EHCI_PHY_ORION: @@ -317,9 +360,19 @@ static int __exit ehci_orion_drv_remove(struct platform_device *pdev) MODULE_ALIAS(platform:orion-ehci); +static const struct of_device_id ehci_orion_dt_ids[] __devinitdata = { + { .compatible = marvell,orion-ehci, }, + {}, +}; +MODULE_DEVICE_TABLE(of, ehci_orion_dt_ids); + static struct platform_driver ehci_orion_driver = { .probe = ehci_orion_drv_probe, .remove = __exit_p(ehci_orion_drv_remove), .shutdown = usb_hcd_platform_shutdown, - .driver.name= orion-ehci, + .driver
[PATCH 2/2] ARM: Kirkwood: Convert all DT boards to EHCI via DT.
Now that the EHCI driver has DT support, drop old style configuration of it and add DT in its place. Since all the boards enable the EHCI, enable it by default in kirkwood.dtsi. Any new boards which don't have USB can specifically disable it. Signed-off-by: Andrew Lunn and...@lunn.ch --- arch/arm/boot/dts/kirkwood.dtsi |8 arch/arm/mach-kirkwood/board-dnskw.c |1 - arch/arm/mach-kirkwood/board-dockstar.c |1 - arch/arm/mach-kirkwood/board-dreamplug.c |1 - arch/arm/mach-kirkwood/board-dt.c |1 + arch/arm/mach-kirkwood/board-goflexnet.c |1 - arch/arm/mach-kirkwood/board-ib62x0.c |1 - arch/arm/mach-kirkwood/board-iconnect.c |1 - arch/arm/mach-kirkwood/board-iomega_ix2_200.c |2 -- arch/arm/mach-kirkwood/board-lsxl.c |1 - arch/arm/mach-kirkwood/board-ts219.c |1 - 11 files changed, 9 insertions(+), 10 deletions(-) diff --git a/arch/arm/boot/dts/kirkwood.dtsi b/arch/arm/boot/dts/kirkwood.dtsi index cef9616..b36bb85 100644 --- a/arch/arm/boot/dts/kirkwood.dtsi +++ b/arch/arm/boot/dts/kirkwood.dtsi @@ -76,6 +76,14 @@ status = okay; }; + ehci@5 { + compatible = marvell,orion-ehci; + reg = 0x5 0x1000; + interrupts = 19; + phy-version = NA; + status = okay; + }; + sata@8 { compatible = marvell,orion-sata; reg = 0x8 0x5000; diff --git a/arch/arm/mach-kirkwood/board-dnskw.c b/arch/arm/mach-kirkwood/board-dnskw.c index 8103961..0fc78a6 100644 --- a/arch/arm/mach-kirkwood/board-dnskw.c +++ b/arch/arm/mach-kirkwood/board-dnskw.c @@ -111,7 +111,6 @@ void __init dnskw_init(void) { kirkwood_mpp_conf(dnskw_mpp_config); - kirkwood_ehci_init(); kirkwood_ge00_init(dnskw_ge00_data); platform_device_register(dnskw_fan_device); diff --git a/arch/arm/mach-kirkwood/board-dockstar.c b/arch/arm/mach-kirkwood/board-dockstar.c index ed35c12..08835fa 100644 --- a/arch/arm/mach-kirkwood/board-dockstar.c +++ b/arch/arm/mach-kirkwood/board-dockstar.c @@ -55,7 +55,6 @@ void __init dockstar_dt_init(void) if (gpio_request(29, USB Power Enable) != 0 || gpio_direction_output(29, 1) != 0) pr_err(can't setup GPIO 29 (USB Power Enable)\n); - kirkwood_ehci_init(); kirkwood_ge00_init(dockstar_ge00_data); } diff --git a/arch/arm/mach-kirkwood/board-dreamplug.c b/arch/arm/mach-kirkwood/board-dreamplug.c index b5a4922..66c5ac8 100644 --- a/arch/arm/mach-kirkwood/board-dreamplug.c +++ b/arch/arm/mach-kirkwood/board-dreamplug.c @@ -64,7 +64,6 @@ void __init dreamplug_init(void) */ kirkwood_mpp_conf(dreamplug_mpp_config); - kirkwood_ehci_init(); kirkwood_ge00_init(dreamplug_ge00_data); kirkwood_ge01_init(dreamplug_ge01_data); kirkwood_sdio_init(dreamplug_mvsdio_data); diff --git a/arch/arm/mach-kirkwood/board-dt.c b/arch/arm/mach-kirkwood/board-dt.c index 30a4caa..16b767e 100644 --- a/arch/arm/mach-kirkwood/board-dt.c +++ b/arch/arm/mach-kirkwood/board-dt.c @@ -33,6 +33,7 @@ struct of_dev_auxdata kirkwood_auxdata_lookup[] __initdata = { OF_DEV_AUXDATA(marvell,orion-wdt, 0xf1020300, orion_wdt, NULL), OF_DEV_AUXDATA(marvell,orion-sata, 0xf108, sata_mv.0, NULL), OF_DEV_AUXDATA(marvell,orion-nand, 0xf400, orion_nand, NULL), + OF_DEV_AUXDATA(marvell,orion-ehci, 0xf105, orion-ehci.0, NULL), {}, }; diff --git a/arch/arm/mach-kirkwood/board-goflexnet.c b/arch/arm/mach-kirkwood/board-goflexnet.c index 52cde7c..6c1e353 100644 --- a/arch/arm/mach-kirkwood/board-goflexnet.c +++ b/arch/arm/mach-kirkwood/board-goflexnet.c @@ -65,7 +65,6 @@ void __init goflexnet_init(void) if (gpio_request(29, USB Power Enable) != 0 || gpio_direction_output(29, 1) != 0) pr_err(can't setup GPIO 29 (USB Power Enable)\n); - kirkwood_ehci_init(); kirkwood_ge00_init(goflexnet_ge00_data); } diff --git a/arch/arm/mach-kirkwood/board-ib62x0.c b/arch/arm/mach-kirkwood/board-ib62x0.c index 4304a44..4baedae 100644 --- a/arch/arm/mach-kirkwood/board-ib62x0.c +++ b/arch/arm/mach-kirkwood/board-ib62x0.c @@ -61,7 +61,6 @@ void __init ib62x0_init(void) */ kirkwood_mpp_conf(ib62x0_mpp_config); - kirkwood_ehci_init(); kirkwood_ge00_init(ib62x0_ge00_data); if (gpio_request(IB62X0_GPIO_POWER_OFF, ib62x0:power:off) == 0 gpio_direction_output(IB62X0_GPIO_POWER_OFF, 0) == 0) diff --git a/arch/arm/mach-kirkwood/board-iconnect.c b/arch/arm/mach-kirkwood/board-iconnect.c index 71a6cc0..b6b89ea 100644 --- a/arch/arm/mach-kirkwood/board-iconnect.c +++ b/arch/arm/mach-kirkwood/board-iconnect.c @@ -46,7 +46,6 @@ void
EHCI: Centralize controller initialization
Hi Alan kisskb is showing up a warning in drivers/usb/host/ehci-orion.c which i think is from a change you made: drivers/usb/host/ehci-orion.c:109:2: warning: passing argument 1 of 'ehci_setup' from incompatible pointer type [enabled by default] diff --git a/drivers/usb/host/ehci-orion.c b/drivers/usb/host/ehci-orion.c index 82de107..3e41123 100644 (file) --- a/drivers/usb/host/ehci-orion.c +++ b/drivers/usb/host/ehci-orion.c @@ -106,21 +106,10 @@ static int ehci_orion_setup(struct usb_hcd *hcd) struct ehci_hcd *ehci = hcd_to_ehci(hcd); int retval; - hcd-has_tt = 1; - - retval = ehci_halt(ehci); - if (retval) - return retval; - - /* -* data structure init -*/ - retval = ehci_init(hcd); + retval = ehci_setup(ehci); if (retval) return retval; ehci_setup() is being passed a struct ehci_hcd *, but: 811 static int ehci_setup(struct usb_hcd *hcd) 812 { 813 struct ehci_hcd *ehci = hcd_to_ehci(hcd); 814 int retval; it seems to expect a struct usb_hcd * Andrew -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html