Re: [PATCH] [v7] net: emac: emac gigabit ethernet controller driver

2016-08-18 Thread Timur Tabi
Florian Fainelli wrote: There is a helper function to obtain the platform device associated with a device_node: of_find_device_by_node. Thank you, this is exactly what I needed. -- Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies,

Re: [PATCH] [v7] net: emac: emac gigabit ethernet controller driver

2016-08-18 Thread Florian Fainelli
On August 17, 2016 9:19:23 PM PDT, Timur Tabi wrote: >Florian Fainelli wrote: >> The larger issue is that the emac_sgmii node in the form you posted >> is going to be backed by a platform device in Linux while you want a >> PHY device with a reg property that describes a

Re: [PATCH] [v7] net: emac: emac gigabit ethernet controller driver

2016-08-17 Thread Timur Tabi
Florian Fainelli wrote: The larger issue is that the emac_sgmii node in the form you posted is going to be backed by a platform device in Linux while you want a PHY device with a reg property that describes a MDIO address (#address-cells = 1, #size-cells = 0). But how do I get the platform

Re: [PATCH] [v7] net: emac: emac gigabit ethernet controller driver

2016-08-17 Thread Florian Fainelli
On August 17, 2016 8:27:19 PM PDT, Timur Tabi wrote: >Florian Fainelli wrote: > >>> emac_sgmii: ethernet-phy@410400 { >>> compatible = "qcom,qdf2432-emac-phy"; >>> reg = <0x0 0x00410400 0x0 0x100>; >>> interrupts = <0 0x104 0>; >>> }; >>>

Re: [PATCH] [v7] net: emac: emac gigabit ethernet controller driver

2016-08-17 Thread Timur Tabi
Florian Fainelli wrote: emac_sgmii: ethernet-phy@410400 { compatible = "qcom,qdf2432-emac-phy"; reg = <0x0 0x00410400 0x0 0x100>; interrupts = <0 0x104 0>; }; Is this register range relative to the emac0 node here, or is this really a separate node,

Re: [PATCH] [v7] net: emac: emac gigabit ethernet controller driver

2016-08-17 Thread Florian Fainelli
On 08/17/2016 03:32 PM, Timur Tabi wrote: > Timur Tabi wrote: >>> >>> Nothing prevents you from detailing in ACPI or DT sub-components of a >>> larger HW block, if there is such a desire, but just make it in a way >>> that it looks like what would be done for e.g: separate discrete parts, >>> only

Re: [PATCH] [v7] net: emac: emac gigabit ethernet controller driver

2016-08-17 Thread Timur Tabi
Timur Tabi wrote: Nothing prevents you from detailing in ACPI or DT sub-components of a larger HW block, if there is such a desire, but just make it in a way that it looks like what would be done for e.g: separate discrete parts, only the parenting of nodes would change. So instead of just

Re: [PATCH] [v7] net: emac: emac gigabit ethernet controller driver

2016-08-17 Thread Timur Tabi
Florian Fainelli wrote: If it has self identification, and the MAC is capable of knowing what kind of PHY it is internally bundled with, why does it matter to represent that specific piece of information in Device Tree or ACPI? That's the thing - the MAC is not capable of knowing. From a

Re: [PATCH] [v7] net: emac: emac gigabit ethernet controller driver

2016-08-17 Thread Florian Fainelli
On 08/16/2016 02:37 PM, Timur Tabi wrote: > Al Stone wrote: >> Does the ACPI portion of the driver*have* to know about the PHY? In >> general, >> the ACPI assumption on ARM [**] is that those have all been set up >> before we >> get to the kernel. So, does it need to be visible to the ACPI part

Re: [PATCH] [v7] net: emac: emac gigabit ethernet controller driver

2016-08-17 Thread Timur Tabi
Timur Tabi wrote: The question, how should adpt->phy.version be initialized on device tree and ACPI platforms? The reason why this is confusing is because there are conflicting perspectives of this "internal PHY". It both is and is not part of the EMAC. It is a separate block in the chip,

Re: [PATCH] [v7] net: emac: emac gigabit ethernet controller driver

2016-08-16 Thread Timur Tabi
Al Stone wrote: Does the ACPI portion of the driver*have* to know about the PHY? In general, the ACPI assumption on ARM [**] is that those have all been set up before we get to the kernel. So, does it need to be visible to the ACPI part of the driver at all? Yes, the driver supports both

Re: [PATCH] [v7] net: emac: emac gigabit ethernet controller driver

2016-08-16 Thread Al Stone
On 08/16/2016 07:39 AM, Timur Tabi wrote: > Rob Herring wrote: > >>> In ACPI, the equivalent to a compatible string is the HID, which is QCOM8070 >>> for the EMAC. The problem is that it's very difficult, if not impossible, >>> to create new HIDs for different versions of the same device. >> >>

Re: [PATCH] [v7] net: emac: emac gigabit ethernet controller driver

2016-08-16 Thread Timur Tabi
Rob Herring wrote: In ACPI, the equivalent to a compatible string is the HID, which is QCOM8070 for the EMAC. The problem is that it's very difficult, if not impossible, to create new HIDs for different versions of the same device. Different versions are different devices IMO. Not that I

Re: [PATCH] [v7] net: emac: emac gigabit ethernet controller driver

2016-08-16 Thread Rob Herring
On Mon, Aug 15, 2016 at 3:06 PM, Timur Tabi wrote: > + Rafael and Al, for ACPI help. > > Rob Herring wrote: >>> >>> >+Optional properties: >>> >+- phy-version : the version of the integrated emac phy, either 1 or 2. > > >> Sounds like 2 different h/w. The compatible property

Re: [PATCH] [v7] net: emac: emac gigabit ethernet controller driver

2016-08-15 Thread Timur Tabi
+ Rafael and Al, for ACPI help. Rob Herring wrote: >+Optional properties: >+- phy-version : the version of the integrated emac phy, either 1 or 2. Sounds like 2 different h/w. The compatible property should distinguish this. So I implemented this in v8 of my driver, but it is causing

Re: [PATCH] [v7] net: emac: emac gigabit ethernet controller driver

2016-08-11 Thread Timur Tabi
Timur Tabi wrote: The hang occurs with this loop in napi_disable(): while (test_and_set_bit(NAPI_STATE_SCHED, >state)) msleep(1); This loop never exits. Can you give me a clue as to what could be the reason? I figured it out. I cannot call napi_disable() twice in a row.

Re: [PATCH] [v7] net: emac: emac gigabit ethernet controller driver

2016-08-11 Thread Timur Tabi
Timur Tabi wrote: The hang occurs with this loop in napi_disable(): while (test_and_set_bit(NAPI_STATE_SCHED, >state)) msleep(1); This loop never exits. Can you give me a clue as to what could be the reason? I figured it out. I cannot call napi_disable() twice in a row.

Re: [PATCH] [v7] net: emac: emac gigabit ethernet controller driver

2016-08-11 Thread Timur Tabi
Florian Fainelli wrote: netif_stop_queue(netdev); > napi_disable(>rx_q.napi); > >I cannot call just napi_disable() in emac_change_mtu(), because when I >then call emac_mac_down(), the first thing it does is call >netif_stop_queue(), and that's when I timeout/hang. Whatever

Re: [PATCH] [v7] net: emac: emac gigabit ethernet controller driver

2016-08-10 Thread Florian Fainelli
On 08/10/2016 09:38 AM, Timur Tabi wrote: > Florian Fainelli wrote: >>> >Is there an easy way for me to stop the RX path before I set >>> rxbuf_size? >>> > Some netif_xxx function I can call? >> napi_disable() should take care of that. > > It appears that if I call netif_stop_queue() *afer*

Re: [PATCH] [v7] net: emac: emac gigabit ethernet controller driver

2016-08-10 Thread Timur Tabi
Florian Fainelli wrote: >Is there an easy way for me to stop the RX path before I set rxbuf_size? > Some netif_xxx function I can call? napi_disable() should take care of that. It appears that if I call netif_stop_queue() *afer* calling napi_disable(), I get a hang and/or TX timeout. Since

Re: [PATCH] [v7] net: emac: emac gigabit ethernet controller driver

2016-08-09 Thread Florian Fainelli
On 08/09/2016 06:09 PM, Timur Tabi wrote: > Florian Fainelli wrote: > >> nr_frags can't be bigger than MAX_SKB_FRAGS, hence these checks all >> other drivers do against 1 + MAX_SKB_FRAGS. > > Doh, I just realized something. emac_mac_tx_buf_send() just needs to > make sure that there's enough

Re: [PATCH] [v7] net: emac: emac gigabit ethernet controller driver

2016-08-09 Thread Timur Tabi
Florian Fainelli wrote: nr_frags can't be bigger than MAX_SKB_FRAGS, hence these checks all other drivers do against 1 + MAX_SKB_FRAGS. Doh, I just realized something. emac_mac_tx_buf_send() just needs to make sure that there's enough room for ONE skb. For some reason I thought it had to

Re: [PATCH] [v7] net: emac: emac gigabit ethernet controller driver

2016-08-09 Thread Timur Tabi
Timur Tabi wrote: I used to work on PowerPC, so I respect making things work for both endians. However, even I think that this is overkill for my driver. First, there's no way this driver will ever be used on a big-endian system. Second, I'm pretty sure there are lots of places that would need

Re: [PATCH] [v7] net: emac: emac gigabit ethernet controller driver

2016-08-09 Thread Florian Fainelli
On 08/09/2016 11:25 AM, Timur Tabi wrote: > I need some help figuring that out. Like I said, I didn't write this > driver initially, so there are parts that I don't really understand. I > copied the above code from other drivers, but after studying your > question, I think I understand what

Re: [PATCH] [v7] net: emac: emac gigabit ethernet controller driver

2016-08-09 Thread Rob Herring
On Tue, Aug 9, 2016 at 1:25 PM, Timur Tabi wrote: > Lino Sanfilippo wrote: > >>> +/* Fill up transmit descriptors */ >>> +static void emac_tx_fill_tpd(struct emac_adapter *adpt, >>> +struct emac_tx_queue *tx_q, struct sk_buff >>> *skb, >>> +

Re: [PATCH] [v7] net: emac: emac gigabit ethernet controller driver

2016-08-09 Thread Timur Tabi
Lino Sanfilippo wrote: +/* Fill up transmit descriptors */ +static void emac_tx_fill_tpd(struct emac_adapter *adpt, +struct emac_tx_queue *tx_q, struct sk_buff *skb, +struct emac_tpd *tpd) +{ + u16 nr_frags =

Re: [PATCH] [v7] net: emac: emac gigabit ethernet controller driver

2016-08-05 Thread Timur Tabi
Rob Herring wrote: Sounds like 2 different h/w. The compatible property should distinguish this. So I changed it to this: - compatible : Should be "qcom,fsm9900-emac" or "qcom,qdf2432-emac", depending on the version of the internal PHY. "qcom,fsm9900-emac" is for v1, and

Re: [PATCH] [v7] net: emac: emac gigabit ethernet controller driver

2016-08-04 Thread Lino Sanfilippo
Hi Timur, On 03.08.2016 22:12, Timur Tabi wrote: > +/* Fill up transmit descriptors */ > +static void emac_tx_fill_tpd(struct emac_adapter *adpt, > + struct emac_tx_queue *tx_q, struct sk_buff *skb, > + struct emac_tpd *tpd) > +{ > + u16

Re: [PATCH] [v7] net: emac: emac gigabit ethernet controller driver

2016-08-04 Thread Timur Tabi
Rob Herring wrote: >+- phy-version : the version of the integrated emac phy, either 1 or 2. Sounds like 2 different h/w. The compatible property should distinguish this. Well, the MAC parts of the EMAC is the same. I could do qcom,fsm9900-emac for v1 and qcom,qdf2432-emac for v2. I can't

Re: [PATCH] [v7] net: emac: emac gigabit ethernet controller driver

2016-08-04 Thread Rob Herring
On Wed, Aug 03, 2016 at 03:12:23PM -0500, Timur Tabi wrote: > Add supports for ethernet controller HW on Qualcomm Technologies, Inc. SoC. > This driver supports the following features: > 1) Checksum offload. > 2) Interrupt coalescing support. > 3) SGMII phy. > 4) phylib interface for external phy

[PATCH] [v7] net: emac: emac gigabit ethernet controller driver

2016-08-03 Thread Timur Tabi
Add supports for ethernet controller HW on Qualcomm Technologies, Inc. SoC. This driver supports the following features: 1) Checksum offload. 2) Interrupt coalescing support. 3) SGMII phy. 4) phylib interface for external phy Based on original work by Niranjana Vishwanathapura