Re: [PATCH 1/2] [v4] net: emac: emac gigabit ethernet controller driver

2016-06-01 Thread Timur Tabi
Florian Fainelli wrote: +/* Transmit the packet */ >+static int emac_start_xmit(struct sk_buff *skb, struct net_device *netdev) >+{ >+ struct emac_adapter *adpt = netdev_priv(netdev); >+ >+ return emac_mac_tx_buf_send(adpt, >tx_q, skb); I would inline emac_mac_tx_buf_send()'s body here to

Re: [PATCH 1/2] [v4] net: emac: emac gigabit ethernet controller driver

2016-05-11 Thread Timur Tabi
Timur Tabi wrote: I think the problem is that the current driver seems to be too eager to start/stop the MAC. Please take a look at emac_work_thread_link_check() at https://lkml.org/lkml/2016/4/13/670. Every time the PHY link goes up, it does this: Never mind, I figured out the problem. I

Re: [PATCH 1/2] [v4] net: emac: emac gigabit ethernet controller driver

2016-05-10 Thread Timur Tabi
Florian Fainelli wrote: The Ethernet MAC should be started in ndo_open() and stopped in ndo_close(), in between, there are link state changes, but you are not supposed to stop or start your Ethernet MAC and its DMA for instance during link change, if that is a HW requirement, your HW is pretty

Re: [PATCH 1/2] [v4] net: emac: emac gigabit ethernet controller driver

2016-05-10 Thread Florian Fainelli
On 05/10/2016 04:18 PM, Timur Tabi wrote: > Florian Fainelli wrote: >> Are you utilizing the PHYLIB APIs properly? You need at least a >> phy_start() to start the PHY state machine, and an adjust_link callback >> to be provided to phy_connect() (or of_phy_connect()) to manage link >> state

Re: [PATCH 1/2] [v4] net: emac: emac gigabit ethernet controller driver

2016-05-10 Thread Timur Tabi
Florian Fainelli wrote: Are you utilizing the PHYLIB APIs properly? You need at least a phy_start() to start the PHY state machine, and an adjust_link callback to be provided to phy_connect() (or of_phy_connect()) to manage link state changes. And that's the very basic minimum here, there could

Re: [PATCH 1/2] [v4] net: emac: emac gigabit ethernet controller driver

2016-04-25 Thread Andrew Lunn
> Does the compatible property of the phy node (for the external phy) > need to list the actual external phy? That is, should it look like > this: > > phy0: ethernet-phy@0 { > compatible = "qcom,fsm9900-emac-phy"; > reg = <0>; > } > > or this: > >

Re: [PATCH 1/2] [v4] net: emac: emac gigabit ethernet controller driver

2016-04-22 Thread Florian Fainelli
On 22/04/16 12:45, Timur Tabi wrote: > Timur Tabi wrote: >>> >> >> So I've done some more research, and I believe that the internal phy is >> not a candidate for phylib, but the external phy (which is a real phy) >> might be. There's no MDIO bus to the internal phy. >> >> Does this mean that I

Re: [PATCH 1/2] [v4] net: emac: emac gigabit ethernet controller driver

2016-04-22 Thread Timur Tabi
Timur Tabi wrote: So I've done some more research, and I believe that the internal phy is not a candidate for phylib, but the external phy (which is a real phy) might be. There's no MDIO bus to the internal phy. Does this mean that I will need to enable a PHY driver, and that driver will

Re: [PATCH 1/2] [v4] net: emac: emac gigabit ethernet controller driver

2016-04-21 Thread Timur Tabi
Florian Fainelli wrote: Well, PHYLIB does prefer using MDIO accesses to "speak" to PHYs, built-in or external, but there is always the option of investing into some custom development with the subsystem to make it play nicely with your HW. So I've done some more research, and I believe that

Re: [PATCH 1/2] [v4] net: emac: emac gigabit ethernet controller driver

2016-04-15 Thread Timur Tabi
Bjorn Andersson wrote: Your driver is a platform driver and it's being probed from DT. As such all this is already taken care of for you, by the core. The listing is for your reference to know why the dma-ranges property would affect your device. Ah, sorry. I misunderstood what you meant by

Re: [PATCH 1/2] [v4] net: emac: emac gigabit ethernet controller driver

2016-04-15 Thread Bjorn Andersson
On Fri 15 Apr 10:00 PDT 2016, Timur Tabi wrote: > Bjorn Andersson wrote: > >For platform devices being populated via from DT you will pass: > >of_platform_bus_create() > > of_platform_device_create_pdata() > > of_dma_configure() > > > >Which calls of_dma_get_range() to acquire this

Re: [PATCH 1/2] [v4] net: emac: emac gigabit ethernet controller driver

2016-04-15 Thread Timur Tabi
Rob Herring wrote: You may only care about the size, but the binding has to handle the more complex case. Here's an example <0x0 0x2 0x0 0x1 0x0> dma address 0 (cell 0) maps to cpu (parent) address 0x2_ (cell 1-2) and the range/size is 4G (cell 3-4). If you have the same base address,

Re: [PATCH 1/2] [v4] net: emac: emac gigabit ethernet controller driver

2016-04-15 Thread Timur Tabi
Bjorn Andersson wrote: For platform devices being populated via from DT you will pass: of_platform_bus_create() of_platform_device_create_pdata() of_dma_configure() Which calls of_dma_get_range() to acquire this information from the dma-ranges property and set up the dma ops and

Re: [PATCH 1/2] [v4] net: emac: emac gigabit ethernet controller driver

2016-04-15 Thread Bjorn Andersson
On Thu 14 Apr 16:34 PDT 2016, Timur Tabi wrote: [..] > So I think the solution is to create a device tree (and ACPI) property that > holds the mask. > > dma-mask = <0 0x>; > > or > > dma-mask = <0x 0x>; > > The driver will then do this: > > u64

Re: [PATCH 1/2] [v4] net: emac: emac gigabit ethernet controller driver

2016-04-15 Thread Rob Herring
On Fri, Apr 15, 2016 at 10:44 AM, Timur Tabi wrote: > Rob Herring wrote: >>> >>> > >>> > dma-mask = <0 0x>; >>> > >>> >or >>> > >>> > dma-mask = <0x 0x>; >> >> No. See dma-ranges. > > > How exactly should I use dma-ranges? I can't

Re: [PATCH 1/2] [v4] net: emac: emac gigabit ethernet controller driver

2016-04-15 Thread Timur Tabi
Rob Herring wrote: > > dma-mask = <0 0x>; > >or > > dma-mask = <0x 0x>; No. See dma-ranges. How exactly should I use dma-ranges? I can't find any other drivers that queries that property and uses the result to call dma_set_mask. I thought the

Re: [PATCH 1/2] [v4] net: emac: emac gigabit ethernet controller driver

2016-04-15 Thread Rob Herring
On Thu, Apr 14, 2016 at 6:34 PM, Timur Tabi wrote: > Vikram Sethi wrote: >> retval = dma_coerce_mask_and_coherent(>dev, >> DMA_BIT_MASK(64)); >> if (retval) { >> dev_err(>dev, "failed to set DMA mask err %d\n", >> retval);

Re: [PATCH 1/2] [v4] net: emac: emac gigabit ethernet controller driver

2016-04-14 Thread Timur Tabi
Vikram Sethi wrote: >> retval = dma_coerce_mask_and_coherent(>dev, DMA_BIT_MASK(64)); >> if (retval) { >> dev_err(>dev, "failed to set DMA mask err %d\n", retval); >> goto err_res; >> } How can you set the mask to 64 bits when the EMAC IP on FSM9900 and QDF2432 can

Re: [PATCH 1/2] [v4] net: emac: emac gigabit ethernet controller driver

2016-04-14 Thread Vikram Sethi
A couple of clarifications on the SGMII internal PHY and the DMA capability of the EMAC inline. On 04/14/2016 04:19 PM, Florian Fainelli wrote: > On 14/04/16 13:19, Timur Tabi wrote: >> Florian Fainelli wrote: >>> On 13/04/16 10:59, Timur Tabi wrote: From: Gilad Avidov

Re: [PATCH 1/2] [v4] net: emac: emac gigabit ethernet controller driver

2016-04-14 Thread Florian Fainelli
On 14/04/16 13:19, Timur Tabi wrote: > Florian Fainelli wrote: >> On 13/04/16 10:59, Timur Tabi wrote: >>> From: Gilad Avidov >>> >>> Add supports for ethernet controller HW on Qualcomm Technologies, >>> Inc. SoC. >>> This driver supports the following features: >>> 1)

Re: [PATCH 1/2] [v4] net: emac: emac gigabit ethernet controller driver

2016-04-14 Thread Timur Tabi
Florian Fainelli wrote: On 13/04/16 10:59, Timur Tabi wrote: From: Gilad Avidov Add supports for ethernet controller HW on Qualcomm Technologies, Inc. SoC. This driver supports the following features: 1) Checksum offload. 2) Runtime power management support. 3)

Re: [PATCH 1/2] [v4] net: emac: emac gigabit ethernet controller driver

2016-04-14 Thread Rob Herring
On Thu, Apr 14, 2016 at 11:47 AM, Timur Tabi wrote: > Rob Herring wrote: > >>> @@ -0,0 +1,65 @@ >>> +Qualcomm EMAC Gigabit Ethernet Controller >>> + >>> +Required properties: >>> +- compatible : Should be "qcom,emac". >> >> >> Come on... Can you guess what I'm going to say

Re: [PATCH 1/2] [v4] net: emac: emac gigabit ethernet controller driver

2016-04-14 Thread Timur Tabi
Rob Herring wrote: @@ -0,0 +1,65 @@ +Qualcomm EMAC Gigabit Ethernet Controller + +Required properties: +- compatible : Should be "qcom,emac". Come on... Can you guess what I'm going to say here. Ooops, I missed that one. +- reg : Offset and length of the register regions for the device

Re: [PATCH 1/2] [v4] net: emac: emac gigabit ethernet controller driver

2016-04-14 Thread Rob Herring
On Wed, Apr 13, 2016 at 12:59:52PM -0500, Timur Tabi wrote: > From: Gilad Avidov > > Add supports for ethernet controller HW on Qualcomm Technologies, Inc. SoC. > This driver supports the following features: > 1) Checksum offload. > 2) Runtime power management support. >

Re: [PATCH 1/2] [v4] net: emac: emac gigabit ethernet controller driver

2016-04-14 Thread Rob Herring
On Wed, Apr 13, 2016 at 02:31:25PM -0500, Timur Tabi wrote: > kbuild test robot wrote: > > > >drivers/net/ethernet/qualcomm/emac/emac-mac.c: In function 'emac_mac_up': > >drivers/net/ethernet/qualcomm/emac/emac-mac.c:1076:9: warning: large > >integer implicitly truncated to unsigned

Re: [PATCH 1/2] [v4] net: emac: emac gigabit ethernet controller driver

2016-04-13 Thread kbuild test robot
Hi Gilad, [auto build test WARNING on net/master] [also build test WARNING on v4.6-rc3 next-20160413] [if your patch is applied to the wrong git tree, please drop us a note to help improving the system] url:

Re: [PATCH 1/2] [v4] net: emac: emac gigabit ethernet controller driver

2016-04-13 Thread Florian Fainelli
On 13/04/16 10:59, Timur Tabi wrote: > From: Gilad Avidov > > Add supports for ethernet controller HW on Qualcomm Technologies, Inc. SoC. > This driver supports the following features: > 1) Checksum offload. > 2) Runtime power management support. > 3) Interrupt coalescing

Re: [PATCH 1/2] [v4] net: emac: emac gigabit ethernet controller driver

2016-04-13 Thread Bjørn Mork
Timur Tabi writes: > Shanker Donthineni wrote: >> drivers/net/ethernet/qualcomm/emac/emac-mac.c: In function >> 'emac_mac_up': >>drivers/net/ethernet/qualcomm/emac/emac-mac.c:1076:9: warning: >>large integer implicitly truncated to

Re: [PATCH 1/2] [v4] net: emac: emac gigabit ethernet controller driver

2016-04-13 Thread Timur Tabi
Shanker Donthineni wrote: >> drivers/net/ethernet/qualcomm/emac/emac-mac.c: In function 'emac_mac_up': >>drivers/net/ethernet/qualcomm/emac/emac-mac.c:1076:9: warning: large integer implicitly truncated to unsigned type [-Woverflow] >> writel(~DIS_INT, adpt->base +

Re: [PATCH 1/2] [v4] net: emac: emac gigabit ethernet controller driver

2016-04-13 Thread Shanker Donthineni
On 04/13/2016 02:31 PM, Timur Tabi wrote: > kbuild test robot wrote: >> >> drivers/net/ethernet/qualcomm/emac/emac-mac.c: In function 'emac_mac_up': >>drivers/net/ethernet/qualcomm/emac/emac-mac.c:1076:9: warning: large >>integer implicitly truncated to unsigned type [-Woverflow]

Re: [PATCH 1/2] [v4] net: emac: emac gigabit ethernet controller driver

2016-04-13 Thread Timur Tabi
kbuild test robot wrote: drivers/net/ethernet/qualcomm/emac/emac-mac.c: In function 'emac_mac_up': >>drivers/net/ethernet/qualcomm/emac/emac-mac.c:1076:9: warning: large integer implicitly truncated to unsigned type [-Woverflow] writel(~DIS_INT, adpt->base + EMAC_INT_STATUS);

Re: [PATCH 1/2] [v4] net: emac: emac gigabit ethernet controller driver

2016-04-13 Thread kbuild test robot
Hi Gilad, [auto build test WARNING on net/master] [also build test WARNING on v4.6-rc3 next-20160413] [if your patch is applied to the wrong git tree, please drop us a note to help improving the system] url: