Re: [RFC 1/1] MicroSemi Switchtec management interface driver
On 19/12/16 10:02 AM, Keith Busch wrote: > Some of this would be simplified if you use the managed device API's: > devm_request_irq, pcim_enable_device, pcim_iomap, etc... Thanks Keith, I'll look into using those. Logan -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC 1/1] MicroSemi Switchtec management interface driver
On Sat, Dec 17, 2016 at 10:09:22AM -0700, Logan Gunthorpe wrote: > Microsemi's "Switchtec" line of PCI switch devices is already > supported by the kernel with standard PCI switch drivers. However, the > Switchtec device advertises a special management endpoint which > enables some additional functionality. This includes: > > * Packet and Byte Counters > * 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. Currently no ioctls have been implemented but a couple > may be added in a later revision. > > A short text file is provided which documents the switchtec driver > and outlines the semantics of using the char device. Some of this would be simplified if you use the managed device API's: devm_request_irq, pcim_enable_device, pcim_iomap, etc... -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC 1/1] MicroSemi Switchtec management interface driver
On Sun, Dec 18, 2016 at 10:20:47AM -0700, Logan Gunthorpe wrote:
> Hi Greg,
>
> Thanks for the quick review!
>
> On 18/12/16 12:51 AM, Greg Kroah-Hartman wrote:
> > On Sat, Dec 17, 2016 at 10:09:22AM -0700, Logan Gunthorpe wrote:
> > > +struct switchtec_dev {
> > > + struct pci_dev *pdev;
> > > + struct msix_entry *msix;
> > > + struct device *dev;
> > > + struct kref kref;
> >
> > Why do you have a pointer to a device, yet a kref as well? Just have
> > this structure embed a 'struct device' in itself, like you did for a
> > kref, and you will be fine. Otherwise you are dealing with two
> > different sets of reference counting here, for no good reason.
>
> Ok, understood. I had referenced the device dax driver which did it this way
> in 4.8 but looks like it was changed to the way you suggest in 4.9.
>
> > > +#define stdev_pdev(stdev) ((stdev)->pdev)
> > > +#define stdev_pdev_dev(stdev) (&stdev_pdev(stdev)->dev)
> > > +#define stdev_name(stdev) pci_name(stdev_pdev(stdev))
> > > +#define stdev_dev(stdev) ((stdev)->dev)
> >
> > Ick, just open code these please. That's a huge hint your use of the
> > driver model is not correct :)
>
> Ok, will do. For reference, I was copying
>
> drivers/ntb/hw/intel/ntb_hw_intel.h
>
> which does a similar thing.
No need to copy bad code, I suggest fixing that up as well :)
thanks,
greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC 1/1] MicroSemi Switchtec management interface driver
Hi Greg,
Thanks for the quick review!
On 18/12/16 12:51 AM, Greg Kroah-Hartman wrote:
On Sat, Dec 17, 2016 at 10:09:22AM -0700, Logan Gunthorpe wrote:
+struct switchtec_dev {
+ struct pci_dev *pdev;
+ struct msix_entry *msix;
+ struct device *dev;
+ struct kref kref;
Why do you have a pointer to a device, yet a kref as well? Just have
this structure embed a 'struct device' in itself, like you did for a
kref, and you will be fine. Otherwise you are dealing with two
different sets of reference counting here, for no good reason.
Ok, understood. I had referenced the device dax driver which did it this
way in 4.8 but looks like it was changed to the way you suggest in 4.9.
+#define stdev_pdev(stdev) ((stdev)->pdev)
+#define stdev_pdev_dev(stdev) (&stdev_pdev(stdev)->dev)
+#define stdev_name(stdev) pci_name(stdev_pdev(stdev))
+#define stdev_dev(stdev) ((stdev)->dev)
Ick, just open code these please. That's a huge hint your use of the
driver model is not correct :)
Ok, will do. For reference, I was copying
drivers/ntb/hw/intel/ntb_hw_intel.h
which does a similar thing.
Logan
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC 1/1] MicroSemi Switchtec management interface driver
On Sat, Dec 17, 2016 at 10:09:22AM -0700, Logan Gunthorpe wrote:
> +struct switchtec_dev {
> + struct pci_dev *pdev;
> + struct msix_entry *msix;
> + struct device *dev;
> + struct kref kref;
Why do you have a pointer to a device, yet a kref as well? Just have
this structure embed a 'struct device' in itself, like you did for a
kref, and you will be fine. Otherwise you are dealing with two
different sets of reference counting here, for no good reason.
> +
> + unsigned int event_irq;
> +
> + void __iomem *mmio;
> + struct mrpc_regs __iomem *mmio_mrpc;
> + struct ntb_info_regs __iomem *mmio_ntb;
> + struct part_cfg_regs __iomem *mmio_part_cfg;
> +
> + /*
> + * The mrpc mutex must be held when accessing the other
> + * mrpc fields
> + */
> + struct mutex mrpc_mutex;
> + struct list_head mrpc_queue;
> + int mrpc_busy;
> + struct work_struct mrpc_work;
> + struct delayed_work mrpc_timeout;
> +};
> +
> +#define stdev_pdev(stdev) ((stdev)->pdev)
> +#define stdev_pdev_dev(stdev) (&stdev_pdev(stdev)->dev)
> +#define stdev_name(stdev) pci_name(stdev_pdev(stdev))
> +#define stdev_dev(stdev) ((stdev)->dev)
Ick, just open code these please. That's a huge hint your use of the
driver model is not correct :)
thanks,
greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC 1/1] MicroSemi Switchtec management interface driver
Microsemi's "Switchtec" line of PCI switch devices is already supported by the kernel with standard PCI switch drivers. However, the Switchtec device advertises a special management endpoint which enables some additional functionality. This includes: * Packet and Byte Counters * 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. Currently no ioctls have been implemented but a couple may be added in a later revision. A short text file is provided which documents the switchtec driver and outlines the semantics of using the char device. A WIP userspace tool which utilizes this interface is available at [1]. This tool takes inspiration (and borrows some code) from nvme-cli [2]. [1] https://github.com/sbates130272/switchtec-user [2] https://github.com/linux-nvme/nvme-cli Signed-off-by: Logan Gunthorpe Signed-off-by: Stephen Bates --- Documentation/switchtec.txt| 54 +++ MAINTAINERS| 9 + drivers/pci/Kconfig| 1 + drivers/pci/Makefile | 1 + drivers/pci/switch/Kconfig | 13 + drivers/pci/switch/Makefile| 1 + drivers/pci/switch/switchtec.c | 824 + drivers/pci/switch/switchtec.h | 119 ++ 8 files changed, 1022 insertions(+) create mode 100644 Documentation/switchtec.txt create mode 100644 drivers/pci/switch/Kconfig create mode 100644 drivers/pci/switch/Makefile create mode 100644 drivers/pci/switch/switchtec.c create mode 100644 drivers/pci/switch/switchtec.h diff --git a/Documentation/switchtec.txt b/Documentation/switchtec.txt new file mode 100644 index 000..04657ce --- /dev/null +++ b/Documentation/switchtec.txt @@ -0,0 +1,54 @@ + +Linux Switchtec Support + + +Microsemi's "Switchtec" line of PCI switch devices is already +supported by the kernel with standard PCI switch drivers. However, the +Switchtec device advertises a special management endpoint which +enables some additional functionality. This includes: + + * Packet and Byte Counters + * Firmware Upgrades + * Event and Error logs + * Querying port link status + * Custom user firmware commands + +The switchtec kernel module implements this functionality. + + + +Interface += + +The primary means of communicating with the Switchtec management firmware is +through the Memory-mapped Remote Procedure Call (MRPC) interface. +Commands are submitted to the interface with a 4-byte command +identifier and up to 1KB of command specific data. The firmware will +respond with a 4 bytes return code and up to 1KB of command specific +data. The interface only processes a single command at a time. + + +Userspace Interface +=== + +The MRPC interface will be exposed to userspace through a simple char +device: /dev/switchtec#, one for each management endpoint in the system. + +The char device has the following semantics: + + * A write must consist of at least 4 bytes and no more than 1028 bytes. + The first four bytes will be interpreted as the command to run and + the remainder will be used as the input data. A write will send the + command to the firmware to begin processing. + + * Each write must be followed by exactly one read. Any double write will + produce an error and any read that doesn't follow a write will + produce an error. + + * A read will block until the firmware completes the command and return + the four bytes of status plus up to 1024 bytes of output data. (The + length will be specified by the size parameter of the read call -- + reading less than 4 bytes will produce an error. + + * The poll call will also be supported for userspace applications that + need to do other things while waiting for the command to complete. diff --git a/MAINTAINERS b/MAINTAINERS index 63cefa6..1e21505 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -9288,6 +9288,15 @@ S: Maintained F: Documentation/devicetree/bindings/pci/aardvark-pci.txt F: drivers/pci/host/pci-aardvark.c +PCI DRIVER FOR MICROSEMI SWITCHTEC +M: Kurt Schwemmer +M: Stephen Bates +M: Logan Gunthorpe +L: linux-...@vger.kernel.org +S: Maintained +F: Documentation/switchtec.txt +F: drivers/pci/switch/switchtec* + PCI DRIVER FOR NVIDIA TEGRA M: Thierry Reding L: linux-te...@vger.kernel.org diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig index 6555eb7..f72e8c5 100644 --- a/drivers/pci/Kconfig +++ b/drivers/pci/Kconfig @@ -133,3 +133,4 @@ config PCI_HYPERV source "drivers/pci/hotplug/Kconfig" source "drivers/pci/host/Kconfig" +source "drivers/pci/switch/Kconfig" diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile index 8db5079..15b46dd 100644 --- a/drivers/pci/Makefile +++ b/drivers/pci/Makef
