Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1321988 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH V3] net: emac: emac gigabit ethernet controller driver Timur Tabi <timur@codeaurora.org> - 2016-01-29 19:30 +0100
    Re: [PATCH V3] net: emac: emac gigabit ethernet controller driver Rob Herring <robh@kernel.org> - 2016-01-29 21:20 +0100
      Re: [PATCH V3] net: emac: emac gigabit ethernet controller driver Timur Tabi <timur@codeaurora.org> - 2016-01-29 21:40 +0100
        Re: [PATCH V3] net: emac: emac gigabit ethernet controller driver Rob Herring <robh@kernel.org> - 2016-01-30 23:00 +0100

#1321988 — Re: [PATCH V3] net: emac: emac gigabit ethernet controller driver

Gilad is no longer working for Qualcomm, so I'm taking over (as best as 
I can) this driver.  Let's just say it's going to be a learning experience.

Rob Herring wrote:
>> diff --git a/Documentation/devicetree/bindings/net/qcom-emac.txt b/Documentation/devicetree/bindings/net/qcom-emac.txt
>> new file mode 100644
>> index 0000000..8d58a40
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/qcom-emac.txt
>> @@ -0,0 +1,68 @@
>> +Qualcomm EMAC Gigabit Ethernet Controller
>> +
>> +Required properties:
>> +- cell-index : EMAC controller instance number.
>> +- compatible : Should be "qcom,emac".
>
> This should be more specific with the SOC name.

The emac is present on a lot of Qualcomm SOCs, and there are only a few 
variations of it.  It's not really SOC-specific, and the hardware 
version can be queried by the driver.

What did you have in mind?  I don't know even have a list of which SOCs 
has an emac, especially since it's mostly on MSM parts, and I work on 
the server SOC.

>> +- reg : Offset and length of the register regions for the device
>> +- reg-names : Register region names referenced in 'reg' above.
>> +	Required register resource entries are:
>> +	"base"   : EMAC controller base register block.
>> +	"csr"    : EMAC wrapper register block.
>> +	Optional register resource entries are:
>> +	"ptp"    : EMAC PTP (1588) register block.
>> +		   Required if 'qcom,emac-tstamp-en' is present.
>> +	"sgmii"  : EMAC SGMII PHY register block.
>> +- interrupts : Interrupt numbers used by this controller
>> +- interrupt-names : Interrupt resource names referenced in 'interrupts' above.
>> +	Required interrupt resource entries are:
>> +	"emac_core0"   : EMAC core0 interrupt.
>> +	"sgmii_irq"   : EMAC SGMII interrupt.
>> +- qcom,emac-gpio-mdc  : GPIO pin number of the MDC line of MDIO bus.
>> +- qcom,emac-gpio-mdio : GPIO pin number of the MDIO line of MDIO bus.
>
> Use the standard binding for GPIO controlled MDIO bus.

I'm not familiar with that one.  Are you talking about 
bindings/net/mdio-gpio.txt?

>> +- phy-addr            : Specifies phy address on MDIO bus.
>> +			Required if the optional property "qcom,no-external-phy"
>> +			is not specified.
>
> Don't you think you will need to know the specific phy device or other
> properties of the phy?

That, I can't answer.  Aren't all MDIO devices basically the same?  It's 
been a while since I've worked on them.

>> +Optional properties:
>> +- qcom,emac-tstamp-en       : Enables the PTP (1588) timestamping feature.
>> +			      Include this only if PTP (1588) timestamping
>> +			      feature is needed. If included, "ptp" register
>> +			      base should be specified.
>
> Isn't this a user enabled feature if the h/w supports it?

Is there a sysfs entry for that?  We were planning on having a similar 
ACPI property.

[toc] | [next] | [standalone]

#1322056

On Fri, Jan 29, 2016 at 12:22 PM, Timur Tabi <timur@codeaurora.org> wrote:
> Gilad is no longer working for Qualcomm, so I'm taking over (as best as I
> can) this driver.  Let's just say it's going to be a learning experience.

Lucky you.

> Rob Herring wrote:
>>>
>>> diff --git a/Documentation/devicetree/bindings/net/qcom-emac.txt
>>> b/Documentation/devicetree/bindings/net/qcom-emac.txt
>>> new file mode 100644
>>> index 0000000..8d58a40
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/net/qcom-emac.txt
>>> @@ -0,0 +1,68 @@
>>> +Qualcomm EMAC Gigabit Ethernet Controller
>>> +
>>> +Required properties:
>>> +- cell-index : EMAC controller instance number.
>>> +- compatible : Should be "qcom,emac".
>>
>>
>> This should be more specific with the SOC name.
>
>
> The emac is present on a lot of Qualcomm SOCs, and there are only a few
> variations of it.  It's not really SOC-specific, and the hardware version
> can be queried by the driver.

Can the integration bugs be queried, too?

Come on, you know how compatible strings work.

> What did you have in mind?  I don't know even have a list of which SOCs has
> an emac, especially since it's mostly on MSM parts, and I work on the server
> SOC.

You only need the one you care about. Figure out the part number
already. Or find the closest MSM part.

>>> +- reg : Offset and length of the register regions for the device
>>> +- reg-names : Register region names referenced in 'reg' above.
>>> +       Required register resource entries are:
>>> +       "base"   : EMAC controller base register block.
>>> +       "csr"    : EMAC wrapper register block.
>>> +       Optional register resource entries are:
>>> +       "ptp"    : EMAC PTP (1588) register block.
>>> +                  Required if 'qcom,emac-tstamp-en' is present.
>>> +       "sgmii"  : EMAC SGMII PHY register block.
>>> +- interrupts : Interrupt numbers used by this controller
>>> +- interrupt-names : Interrupt resource names referenced in 'interrupts'
>>> above.
>>> +       Required interrupt resource entries are:
>>> +       "emac_core0"   : EMAC core0 interrupt.
>>> +       "sgmii_irq"   : EMAC SGMII interrupt.
>>> +- qcom,emac-gpio-mdc  : GPIO pin number of the MDC line of MDIO bus.
>>> +- qcom,emac-gpio-mdio : GPIO pin number of the MDIO line of MDIO bus.
>>
>>
>> Use the standard binding for GPIO controlled MDIO bus.
>
>
> I'm not familiar with that one.  Are you talking about
> bindings/net/mdio-gpio.txt?

Yes.


>>> +- phy-addr            : Specifies phy address on MDIO bus.
>>> +                       Required if the optional property
>>> "qcom,no-external-phy"
>>> +                       is not specified.
>>
>>
>> Don't you think you will need to know the specific phy device or other
>> properties of the phy?
>
>
> That, I can't answer.  Aren't all MDIO devices basically the same?  It's
> been a while since I've worked on them.

No. There was some discussion just this week about needing to require
phy devices to have compatible strings.

>>> +Optional properties:
>>> +- qcom,emac-tstamp-en       : Enables the PTP (1588) timestamping
>>> feature.
>>> +                             Include this only if PTP (1588)
>>> timestamping
>>> +                             feature is needed. If included, "ptp"
>>> register
>>> +                             base should be specified.
>>
>>
>> Isn't this a user enabled feature if the h/w supports it?
>
>
> Is there a sysfs entry for that?  We were planning on having a similar ACPI
> property.

It would be in ethtool I think.

Rob

[toc] | [prev] | [next] | [standalone]

#1322062

Rob Herring wrote:
>> The emac is present on a lot of Qualcomm SOCs, and there are only a few
>> variations of it.  It's not really SOC-specific, and the hardware version
>> can be queried by the driver.
>
> Can the integration bugs be queried, too?
>
> Come on, you know how compatible strings work.

Fine.  I'll figure something out.

>> What did you have in mind?  I don't know even have a list of which SOCs has
>> an emac, especially since it's mostly on MSM parts, and I work on the server
>> SOC.
>
> You only need the one you care about. Figure out the part number
> already. Or find the closest MSM part.

Ironically, the only SOC I actually care about is not supported by this 
driver yet.

>>>> +- qcom,emac-gpio-mdc  : GPIO pin number of the MDC line of MDIO bus.
>>>> +- qcom,emac-gpio-mdio : GPIO pin number of the MDIO line of MDIO bus.
>>>
>>>
>>> Use the standard binding for GPIO controlled MDIO bus.
>>
>> I'm not familiar with that one.  Are you talking about
>> bindings/net/mdio-gpio.txt?
>
> Yes.

Thanks.

>> That, I can't answer.  Aren't all MDIO devices basically the same?  It's
>> been a while since I've worked on them.
>
> No. There was some discussion just this week about needing to require
> phy devices to have compatible strings.

Ok, I'll check it out.

>>> Isn't this a user enabled feature if the h/w supports it?
>>
>>
>> Is there a sysfs entry for that?  We were planning on having a similar ACPI
>> property.
>
> It would be in ethtool I think.

Ah, this driver does not support ethtool yet.

However, based on a cursory look of ethtool, it appears that there's 
only an option to query the current timestamp, but not actually 
enable/disable the feature.  The e1000e driver, for example, just forces 
the feature by default for various chips.  Is there any reason why we 
shouldn't enable it if the hardware supports it?

[toc] | [prev] | [next] | [standalone]

#1322517

On Fri, Jan 29, 2016 at 2:38 PM, Timur Tabi <timur@codeaurora.org> wrote:
> Rob Herring wrote:

[...]

>>>> Isn't this a user enabled feature if the h/w supports it?
>>>
>>>
>>>
>>> Is there a sysfs entry for that?  We were planning on having a similar
>>> ACPI
>>> property.
>>
>>
>> It would be in ethtool I think.
>
>
> Ah, this driver does not support ethtool yet.
>
> However, based on a cursory look of ethtool, it appears that there's only an
> option to query the current timestamp, but not actually enable/disable the
> feature.  The e1000e driver, for example, just forces the feature by default
> for various chips.  Is there any reason why we shouldn't enable it if the
> hardware supports it?

Probably not. You simply ignore the timestamp if you don't care. So
then why do you want a DT property?

Rob

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web