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

Groups > linux.kernel > #1570914 > unrolled thread

Re: [PATCH 1/1] MicroSemi Switchtec management interface 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 1/1] MicroSemi Switchtec management interface driver Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-01-31 18:30 +0100
    Re: [PATCH 1/1] MicroSemi Switchtec management interface driver Logan Gunthorpe <logang@deltatee.com> - 2017-01-31 18:40 +0100
      Re: [PATCH 1/1] MicroSemi Switchtec management interface driver Jonathan Corbet <corbet@lwn.net> - 2017-01-31 18:50 +0100
        Re: [PATCH 1/1] MicroSemi Switchtec management interface driver Logan Gunthorpe <logang@deltatee.com> - 2017-01-31 20:00 +0100
      Re: [PATCH 1/1] MicroSemi Switchtec management interface driver Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-01-31 20:00 +0100
        Re: [PATCH 1/1] MicroSemi Switchtec management interface driver Logan Gunthorpe <logang@deltatee.com> - 2017-01-31 20:30 +0100

#1570914 — Re: [PATCH 1/1] MicroSemi Switchtec management interface driver

On Tue, Jan 31, 2017 at 10:03:24AM -0700, Logan Gunthorpe wrote:
> Microsemi's "Switchtec" line of PCI switch devices is already well
> supported by the kernel with standard PCI switch drivers. However, the
> Switchtec device advertises a special management endpoint with a separate
> PCI function address and class code. This endpoint enables some additional
> functionality which includes:
> 
>  * Packet and Byte Counters
>  * Switch Firmware Upgrades
>  * Event and Error logs
>  * Querying port link status
>  * Custom user firmware commands
> 
> This patch introduces the switchtec kernel module which provides
> PCI driver that exposes a char device. The char device provides
> userspace access to this interface through read, write and (optionally)
> poll calls. A couple of special IOCTLs are provided to:
> 
> * Inform userspace of firmware partition locations
> * Pass event counts and allow userspace to wait on events
> 
> A short text file is provided which documents the switchtec driver,
> outlines the semantics of using the char device and describes the
> IOCTLs.
> 
> The device also exposes a few read-only sysfs attributes which provide
> some device information component names and versions which is provided
> by the hardware. These are documented in
> Documentation/ABI/testing/sysfs-class-switchtec
> 
> A userspace tool and library which utilizes this interface is available
> at [1]. This tool takes inspiration (and borrows some code) from
> nvme-cli [2]. The tool is largely complete at this time but additional
> features may be added in the future.
> 
> [1] https://github.com/sbates130272/switchtec-user
> [2] https://github.com/linux-nvme/nvme-cli
> 
> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
> Signed-off-by: Stephen Bates <stephen.bates@microsemi.com>
> ---
>  Documentation/ABI/testing/sysfs-class-switchtec |   96 ++
>  Documentation/ioctl/ioctl-number.txt            |    1 +
>  Documentation/switchtec.txt                     |   80 ++
>  MAINTAINERS                                     |   11 +
>  drivers/pci/Kconfig                             |    1 +
>  drivers/pci/Makefile                            |    1 +
>  drivers/pci/switch/Kconfig                      |   13 +
>  drivers/pci/switch/Makefile                     |    1 +
>  drivers/pci/switch/switchtec.c                  | 1320 +++++++++++++++++++++++
>  drivers/pci/switch/switchtec.h                  |  266 +++++
>  include/uapi/linux/switchtec_ioctl.h            |  129 +++
>  11 files changed, 1919 insertions(+)

That's one big patch to review, would you want to do that?

Can you break it up into smaller parts?  At least put the documentation
separately, right?

And don't dump a .txt file into Documentation/ anymore, people are
working to move to the newer format.

Also, please rebase against Linus's tree at the least, we can't go back
in time and apply this to the 4.9 kernel tree.

thanks,

greg k-h

[toc] | [next] | [standalone]

#1570923

On 31/01/17 10:26 AM, Greg Kroah-Hartman wrote:
> That's one big patch to review, would you want to do that?

Sorry, will do.

> Can you break it up into smaller parts?  At least put the documentation
> separately, right?

Ha, funny. Last time I sent a patch someone asked for the documentation
to be in the same patch. But I can easily split this up.

> And don't dump a .txt file into Documentation/ anymore, people are
> working to move to the newer format.

Fair. I wasn't sure where a good place to put it was. Any suggestions?

> Also, please rebase against Linus's tree at the least, we can't go back
> in time and apply this to the 4.9 kernel tree.

Will do.

> Why a .h file for a single .c file?

I wanted to keep the hardware defining structs and macros in a separate
file for future expansion. This hardware is also capable of some NTB
functions which may find it's way into the kernel in the future.

> Also, why a whole new directory?

We didn't feel it fit in the pci director which was for standard pci
stuff. We're more than open to other suggestions as to where this code
belongs.

Thanks,

Logan

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

#1570939

On Tue, 31 Jan 2017 10:35:44 -0700
Logan Gunthorpe <logang@deltatee.com> wrote:

> > And don't dump a .txt file into Documentation/ anymore, people are
> > working to move to the newer format.  
> 
> Fair. I wasn't sure where a good place to put it was. Any suggestions?

We're working toward a rational document hierarchy using the sphinx system:

	https://lwn.net/Articles/692704/
	https://lwn.net/Articles/692704/
	https://static.lwn.net/kerneldoc/doc-guide/index.html

The good news is that your switchtec.txt file is already 99% in the RST
format, so there is little or nothing to do there.

The bad news is that we don't quite have a place for it yet.  This is
really user-space developer documentation, and we don't have a sub-book
for that.  I expect that to change pretty soon, and I might even toss
together a bare beginning for 4.11, but that hasn't happened yet.

If you would like to learn the ropes and make one, that would be more than
great :)  Alternatively, I'd suggest just leaving the document as-is, and
I'll put it toward the top of the list of things to move into place once I
get that book started.

Thanks,

jon

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

#1571004

On 31/01/17 10:49 AM, Jonathan Corbet wrote:
> The good news is that your switchtec.txt file is already 99% in the RST
> format, so there is little or nothing to do there.
> 
> The bad news is that we don't quite have a place for it yet.  This is
> really user-space developer documentation, and we don't have a sub-book
> for that.  I expect that to change pretty soon, and I might even toss
> together a bare beginning for 4.11, but that hasn't happened yet.
> 
> If you would like to learn the ropes and make one, that would be more than
> great :)  Alternatively, I'd suggest just leaving the document as-is, and
> I'll put it toward the top of the list of things to move into place once I
> get that book started.

Thanks Jon. I took a look at the documentation and it doesn't look too
difficult. But I don't really feel qualified to make some of the
decisions necessary to create the new book. (ie names, locations, etc.)
And I don't really want that churn in this patchset. So we'll leave it
where it is for now.

I have cleaned up some of the RST format for v2 though.

Logan

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

#1570994

On Tue, Jan 31, 2017 at 10:35:44AM -0700, Logan Gunthorpe wrote:
> 
> 
> On 31/01/17 10:26 AM, Greg Kroah-Hartman wrote:
> > That's one big patch to review, would you want to do that?
> 
> Sorry, will do.
> 
> > Can you break it up into smaller parts?  At least put the documentation
> > separately, right?
> 
> Ha, funny. Last time I sent a patch someone asked for the documentation
> to be in the same patch. But I can easily split this up.

Sorry, it was probably me :)

> > And don't dump a .txt file into Documentation/ anymore, people are
> > working to move to the newer format.
> 
> Fair. I wasn't sure where a good place to put it was. Any suggestions?

Why do you need this?  Wherever you put it, it should be built as part
of the online kernel documentation.  Who is the audience for this
documentation?

> > Also, please rebase against Linus's tree at the least, we can't go back
> > in time and apply this to the 4.9 kernel tree.
> 
> Will do.
> 
> > Why a .h file for a single .c file?
> 
> I wanted to keep the hardware defining structs and macros in a separate
> file for future expansion. This hardware is also capable of some NTB
> functions which may find it's way into the kernel in the future.

Do future stuff in the future, no need for that now, right?

Simple is best.

> > Also, why a whole new directory?
> 
> We didn't feel it fit in the pci director which was for standard pci
> stuff. We're more than open to other suggestions as to where this code
> belongs.

I'll leave that up to the PCI maintainer, but just a single .c file in a
subdir seems odd to me.

thanks,

greg k-h

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

#1571023

On 31/01/17 11:57 AM, Greg Kroah-Hartman wrote:
> Sorry, it was probably me :)

Nope, it was Christoph Hellwig. I don't mind changing it. It's just hard
to know what's expected all the time.

> Why do you need this?  Wherever you put it, it should be built as part
> of the online kernel documentation.  Who is the audience for this
> documentation?

Well usually documenting how to use the device is a good thing. Is it
not? The audience would be people wanting to make use of the userspace
interface. Though in fairness, the vast majority of people should use
our library. I also thought it would be useful to reviewers to more
quickly understand the interface we are creating. If maintainers want us
to leave it out, we can, but that seems like an odd request.

> Do future stuff in the future, no need for that now, right?
> 
> Simple is best.

Ok, I can push it all into the C file for v2.

> I'll leave that up to the PCI maintainer, but just a single .c file in a
> subdir seems odd to me.

Thanks,

Logan

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web