mbox series

[net-next,0/2] Devlink regions for SJA1105 DSA driver

Message ID 20200921162741.4081710-1-vladimir.oltean@nxp.com
Headers show
Series Devlink regions for SJA1105 DSA driver | expand

Message

Vladimir Oltean Sept. 21, 2020, 4:27 p.m. UTC 
This series exposes the SJA1105 static config as a devlink region. This
can be used for debugging, for example with the sja1105_dump user space
program that I have derived from Andrew Lunn's mv88e6xxx_dump:

https://github.com/vladimiroltean/mv88e6xxx_dump/tree/sja1105

Vladimir Oltean (2):
  net: dsa: sja1105: move devlink param code to sja1105_devlink.c
  net: dsa: sja1105: expose static config as devlink region

 drivers/net/dsa/sja1105/Makefile          |   1 +
 drivers/net/dsa/sja1105/sja1105.h         |  13 +-
 drivers/net/dsa/sja1105/sja1105_devlink.c | 236 ++++++++++++++++++++++
 drivers/net/dsa/sja1105/sja1105_main.c    | 105 +---------
 drivers/net/dsa/sja1105/sja1105_spi.c     |   5 +-
 5 files changed, 254 insertions(+), 106 deletions(-)
 create mode 100644 drivers/net/dsa/sja1105/sja1105_devlink.c

Comments

Andrew Lunn Sept. 21, 2020, 5:12 p.m. UTC | #1 
On Mon, Sep 21, 2020 at 07:27:39PM +0300, Vladimir Oltean wrote:
> This series exposes the SJA1105 static config as a devlink region. This
> can be used for debugging, for example with the sja1105_dump user space
> program that I have derived from Andrew Lunn's mv88e6xxx_dump:
> 
> https://github.com/vladimiroltean/mv88e6xxx_dump/tree/sja1105

Maybe i should rename the project dsa_dump?

      Andrew
Vladimir Oltean Sept. 21, 2020, 6:12 p.m. UTC | #2 
On Mon, Sep 21, 2020 at 07:12:32PM +0200, Andrew Lunn wrote:
> On Mon, Sep 21, 2020 at 07:27:39PM +0300, Vladimir Oltean wrote:
> > This series exposes the SJA1105 static config as a devlink region. This
> > can be used for debugging, for example with the sja1105_dump user space
> > program that I have derived from Andrew Lunn's mv88e6xxx_dump:
> >
> > https://github.com/vladimiroltean/mv88e6xxx_dump/tree/sja1105
>
> Maybe i should rename the project dsa_dump?

I was unsure if you want to maintain that as a larger project.
Florian Fainelli Sept. 21, 2020, 6:44 p.m. UTC | #3 
On 9/21/20 11:12 AM, Vladimir Oltean wrote:
> On Mon, Sep 21, 2020 at 07:12:32PM +0200, Andrew Lunn wrote:
>> On Mon, Sep 21, 2020 at 07:27:39PM +0300, Vladimir Oltean wrote:
>>> This series exposes the SJA1105 static config as a devlink region. This
>>> can be used for debugging, for example with the sja1105_dump user space
>>> program that I have derived from Andrew Lunn's mv88e6xxx_dump:
>>>
>>> https://github.com/vladimiroltean/mv88e6xxx_dump/tree/sja1105
>>
>> Maybe i should rename the project dsa_dump?
> 
> I was unsure if you want to maintain that as a larger project.
> 

Cannot this be part of ethtool or iproute2 at some point and we just
have each driver author submit pretty printers for the registers?
Andrew Lunn Sept. 21, 2020, 7:03 p.m. UTC | #4 
On Mon, Sep 21, 2020 at 11:44:41AM -0700, Florian Fainelli wrote:
> On 9/21/20 11:12 AM, Vladimir Oltean wrote:
> > On Mon, Sep 21, 2020 at 07:12:32PM +0200, Andrew Lunn wrote:
> >> On Mon, Sep 21, 2020 at 07:27:39PM +0300, Vladimir Oltean wrote:
> >>> This series exposes the SJA1105 static config as a devlink region. This
> >>> can be used for debugging, for example with the sja1105_dump user space
> >>> program that I have derived from Andrew Lunn's mv88e6xxx_dump:
> >>>
> >>> https://github.com/vladimiroltean/mv88e6xxx_dump/tree/sja1105
> >>
> >> Maybe i should rename the project dsa_dump?
> > 
> > I was unsure if you want to maintain that as a larger project.
> > 
> 
> Cannot this be part of ethtool or iproute2 at some point and we just
> have each driver author submit pretty printers for the registers?

It would be iproute2, since that is where devlink lives and where most
of the code comes from. I did take some code from ethtool which Vivien
wrote for the port registers.

The code needs refactoring anyway, i was not thinking about others
using it, and there is code in mv88e6xxx_dump which should be shared.

Vladimir, could you implement the devlink DEVLINK_CMD_INFO_GET request
so we know what sort of device is exporting the regions, and hence
which pretty printers are relevant.

   Andrew
Vladimir Oltean Sept. 21, 2020, 7:10 p.m. UTC | #5 
On Mon, Sep 21, 2020 at 09:03:27PM +0200, Andrew Lunn wrote:
> Vladimir, could you implement the devlink DEVLINK_CMD_INFO_GET request
> so we know what sort of device is exporting the regions, and hence
> which pretty printers are relevant.

Should I do that in this series or separately?
Andrew Lunn Sept. 21, 2020, 7:42 p.m. UTC | #6 
On Mon, Sep 21, 2020 at 07:10:09PM +0000, Vladimir Oltean wrote:
> On Mon, Sep 21, 2020 at 09:03:27PM +0200, Andrew Lunn wrote:
> > Vladimir, could you implement the devlink DEVLINK_CMD_INFO_GET request
> > so we know what sort of device is exporting the regions, and hence
> > which pretty printers are relevant.
> 
> Should I do that in this series or separately?

Up to you. I added it as part of the regions patchset. I need to know
which particular device it is in order to decode the registers
correctly.

	Andrew
Vladimir Oltean Sept. 21, 2020, 7:47 p.m. UTC | #7 
On Mon, Sep 21, 2020 at 09:42:10PM +0200, Andrew Lunn wrote:
> On Mon, Sep 21, 2020 at 07:10:09PM +0000, Vladimir Oltean wrote:
> > On Mon, Sep 21, 2020 at 09:03:27PM +0200, Andrew Lunn wrote:
> > > Vladimir, could you implement the devlink DEVLINK_CMD_INFO_GET request
> > > so we know what sort of device is exporting the regions, and hence
> > > which pretty printers are relevant.
> > 
> > Should I do that in this series or separately?
> 
> Up to you. I added it as part of the regions patchset.

I was planning to add that only if I need to resend this series,
otherwise I would go with a follow-on.

> I need to know which particular device it is in order to decode the
> registers correctly.

For sja1105, the device id is part of the static config, so the region
is self explanatory in that regard. But if we create a larger tool then
it makes sense to report the device in DEVLINK_CMD_INFO_GET.

Thanks,
-Vladimir