| Message ID | 20210924110842.6323-1-kelvin.cao@microchip.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Switchtec Fixes and Improvements | expand |
On 2021-09-24 5:08 a.m., kelvin.cao@microchip.com wrote: > From: Kelvin Cao <kelvin.cao@microchip.com> > > Hi, > > Please find a bunch of patches for the switchtec driver collected over the > last few months. > > The first 2 patches fix two minor bugs. Patch 3 updates the method of > getting management VEP instance ID based on a new firmware change. Patch > 4 replaces ENOTSUPP with EOPNOTSUPP to follow the preference of kernel. > And the last patch adds check of event support to align with new firmware > implementation. > > This patchset is based on v5.15-rc2. > > Thanks, > Kelvin > > Kelvin Cao (4): > PCI/switchtec: Error out MRPC execution when no GAS access > PCI/switchtec: Fix a MRPC error status handling issue > PCI/switchtec: Update the way of getting management VEP instance ID > PCI/switchtec: Replace ENOTSUPP with EOPNOTSUPP > > Logan Gunthorpe (1): > PCI/switchtec: Add check of event support > > drivers/pci/switch/switchtec.c | 87 +++++++++++++++++++++++++++------- > include/linux/switchtec.h | 1 + > 2 files changed, 71 insertions(+), 17 deletions(-) Thanks Kelvin! This all looks good to me. For the entire series (except the last patch which I wrote): Reviewed-by: Logan Gunthorpe <logang@deltatee.com> Logan
On Fri, 2021-09-24 at 09:53 -0600, Logan Gunthorpe wrote: > On 2021-09-24 5:08 a.m., kelvin.cao@microchip.com wrote: > > From: Kelvin Cao <kelvin.cao@microchip.com> > > > > Hi, > > > > Please find a bunch of patches for the switchtec driver collected > > over the > > last few months. > > > > The first 2 patches fix two minor bugs. Patch 3 updates the method > > of > > getting management VEP instance ID based on a new firmware change. > > Patch > > 4 replaces ENOTSUPP with EOPNOTSUPP to follow the preference of > > kernel. > > And the last patch adds check of event support to align with new > > firmware > > implementation. > > > > This patchset is based on v5.15-rc2. > > > > Thanks, > > Kelvin > > > > Kelvin Cao (4): > > PCI/switchtec: Error out MRPC execution when no GAS access > > PCI/switchtec: Fix a MRPC error status handling issue > > PCI/switchtec: Update the way of getting management VEP instance > > ID > > PCI/switchtec: Replace ENOTSUPP with EOPNOTSUPP > > > > Logan Gunthorpe (1): > > PCI/switchtec: Add check of event support > > > > drivers/pci/switch/switchtec.c | 87 +++++++++++++++++++++++++++--- > > ---- > > include/linux/switchtec.h | 1 + > > 2 files changed, 71 insertions(+), 17 deletions(-) > > Thanks Kelvin! This all looks good to me. > > For the entire series (except the last patch which I wrote): > > Reviewed-by: Logan Gunthorpe <logang@deltatee.com> > > Logan Thanks for the review! Kelvin
On Fri, Sep 24, 2021 at 11:08:37AM +0000, kelvin.cao@microchip.com wrote: > From: Kelvin Cao <kelvin.cao@microchip.com> > > Hi, > > Please find a bunch of patches for the switchtec driver collected over the > last few months. > > The first 2 patches fix two minor bugs. Patch 3 updates the method of > getting management VEP instance ID based on a new firmware change. Patch > 4 replaces ENOTSUPP with EOPNOTSUPP to follow the preference of kernel. > And the last patch adds check of event support to align with new firmware > implementation. > > This patchset is based on v5.15-rc2. > > Thanks, > Kelvin > > Kelvin Cao (4): > PCI/switchtec: Error out MRPC execution when no GAS access > PCI/switchtec: Fix a MRPC error status handling issue > PCI/switchtec: Update the way of getting management VEP instance ID > PCI/switchtec: Replace ENOTSUPP with EOPNOTSUPP > > Logan Gunthorpe (1): > PCI/switchtec: Add check of event support > > drivers/pci/switch/switchtec.c | 87 +++++++++++++++++++++++++++------- > include/linux/switchtec.h | 1 + > 2 files changed, 71 insertions(+), 17 deletions(-) Applied with Logan's reviewed-by to pci/switchtec for v5.16, thanks! I tweaked the comment spacing in "PCI/switchtec: Error out MRPC execution when no GAS access" so it matches the existing similar comments.
On Mon, 2021-09-27 at 11:39 -0500, Bjorn Helgaas wrote: > On Fri, Sep 24, 2021 at 11:08:37AM +0000, kelvin.cao@microchip.com > wrote: > > From: Kelvin Cao <kelvin.cao@microchip.com> > > > > Hi, > > > > Please find a bunch of patches for the switchtec driver collected > > over the > > last few months. > > > > The first 2 patches fix two minor bugs. Patch 3 updates the method > > of > > getting management VEP instance ID based on a new firmware change. > > Patch > > 4 replaces ENOTSUPP with EOPNOTSUPP to follow the preference of > > kernel. > > And the last patch adds check of event support to align with new > > firmware > > implementation. > > > > This patchset is based on v5.15-rc2. > > > > Thanks, > > Kelvin > > > > Kelvin Cao (4): > > PCI/switchtec: Error out MRPC execution when no GAS access > > PCI/switchtec: Fix a MRPC error status handling issue > > PCI/switchtec: Update the way of getting management VEP instance > > ID > > PCI/switchtec: Replace ENOTSUPP with EOPNOTSUPP > > > > Logan Gunthorpe (1): > > PCI/switchtec: Add check of event support > > > > drivers/pci/switch/switchtec.c | 87 +++++++++++++++++++++++++++--- > > ---- > > include/linux/switchtec.h | 1 + > > 2 files changed, 71 insertions(+), 17 deletions(-) > > Applied with Logan's reviewed-by to pci/switchtec for v5.16, thanks! > > I tweaked the comment spacing in "PCI/switchtec: Error out MRPC > execution when no GAS access" so it matches the existing similar > comments. Thanks for your time! I guess the tweak was made to "PCI/switchtec: Replace ENOTSUPP with EOPNOTSUPP" which also confirmed that "Link:" tag should not be put to a discussion reference URL line in the comment. Kelvin
On Fri, Sep 24, 2021 at 11:08:37AM +0000, kelvin.cao@microchip.com wrote: > From: Kelvin Cao <kelvin.cao@microchip.com> > > Hi, > > Please find a bunch of patches for the switchtec driver collected over the > last few months. Question: Is there a reason this driver should be in drivers/pci/? It doesn't use any internal PCI core interfaces, e.g., it doesn't include drivers/pci/pci.h, and AFAICT it's really just a driver for a PCI device that happens to be a switch. I don't really *care* that it's in drivers/pci; I rely on Kurt and Logan to review changes. The only problem it presents for me is that I have to write merge commit logs for the changes. You'd think that would be trivial, but since I don't know much about the driver, it does end up being work for me. Bjorn
On 2021-10-08 11:05 a.m., Bjorn Helgaas wrote: > On Fri, Sep 24, 2021 at 11:08:37AM +0000, kelvin.cao@microchip.com wrote: >> From: Kelvin Cao <kelvin.cao@microchip.com> >> >> Hi, >> >> Please find a bunch of patches for the switchtec driver collected over the >> last few months. > > Question: Is there a reason this driver should be in drivers/pci/? > > It doesn't use any internal PCI core interfaces, e.g., it doesn't > include drivers/pci/pci.h, and AFAICT it's really just a driver for a > PCI device that happens to be a switch. > > I don't really *care* that it's in drivers/pci; I rely on Kurt and > Logan to review changes. The only problem it presents for me is that > I have to write merge commit logs for the changes. You'd think that > would be trivial, but since I don't know much about the driver, it > does end up being work for me. We did discuss this when it was originally merged. The main reason we want it in the PCI tree is so that it's in a sensible spot in the Kconfig hierarchy (under PCI support). Seeing it is still PCI hardware. Dropping it into the miscellaneous devices mess (or similar) is less than desirable. Moreover, it's not like the maintainers for misc have any additional knowledge that would make them better qualified to merge these changes. In fact, I'm sure they'd have less knowledge and we wouldn't have gotten to the bottom of this last issue if it had been a different maintainer. In the future I'll try to be more careful in my reviews to ensure we have a better understanding and clearer commit messages. If there's anything else we can do to make your job easier, please let us know. Thanks, Logan
On Fri, Oct 08, 2021 at 11:23:46AM -0600, Logan Gunthorpe wrote: > On 2021-10-08 11:05 a.m., Bjorn Helgaas wrote: > > On Fri, Sep 24, 2021 at 11:08:37AM +0000, kelvin.cao@microchip.com wrote: > >> From: Kelvin Cao <kelvin.cao@microchip.com> > >> > >> Hi, > >> > >> Please find a bunch of patches for the switchtec driver collected over the > >> last few months. > > > > Question: Is there a reason this driver should be in drivers/pci/? > > > > It doesn't use any internal PCI core interfaces, e.g., it doesn't > > include drivers/pci/pci.h, and AFAICT it's really just a driver for a > > PCI device that happens to be a switch. > > > > I don't really *care* that it's in drivers/pci; I rely on Kurt and > > Logan to review changes. The only problem it presents for me is that > > I have to write merge commit logs for the changes. You'd think that > > would be trivial, but since I don't know much about the driver, it > > does end up being work for me. > > We did discuss this when it was originally merged. Thanks, I thought I remembered talking about it, but didn't bother to dig it up. > The main reason we want it in the PCI tree is so that it's in a sensible > spot in the Kconfig hierarchy (under PCI support). Seeing it is still > PCI hardware. Dropping it into the miscellaneous devices mess (or > similar) is less than desirable. Moreover, it's not like the maintainers > for misc have any additional knowledge that would make them better > qualified to merge these changes. In fact, I'm sure they'd have less > knowledge and we wouldn't have gotten to the bottom of this last issue > if it had been a different maintainer. > > In the future I'll try to be more careful in my reviews to ensure we > have a better understanding and clearer commit messages. If there's > anything else we can do to make your job easier, please let us know. Oh, please don't take this as me complaining about anybody's reviews! I honestly just look for your or Kurt's ack. I think I just need to be a little less fixated on writing the merge commit logs :) Bjorn
From: Kelvin Cao <kelvin.cao@microchip.com> Hi, Please find a bunch of patches for the switchtec driver collected over the last few months. The first 2 patches fix two minor bugs. Patch 3 updates the method of getting management VEP instance ID based on a new firmware change. Patch 4 replaces ENOTSUPP with EOPNOTSUPP to follow the preference of kernel. And the last patch adds check of event support to align with new firmware implementation. This patchset is based on v5.15-rc2. Thanks, Kelvin Kelvin Cao (4): PCI/switchtec: Error out MRPC execution when no GAS access PCI/switchtec: Fix a MRPC error status handling issue PCI/switchtec: Update the way of getting management VEP instance ID PCI/switchtec: Replace ENOTSUPP with EOPNOTSUPP Logan Gunthorpe (1): PCI/switchtec: Add check of event support drivers/pci/switch/switchtec.c | 87 +++++++++++++++++++++++++++------- include/linux/switchtec.h | 1 + 2 files changed, 71 insertions(+), 17 deletions(-)