[PATCH 0/4] Driver for talking to PLX PEX8xxx PCIe switch over I2C

[PATCH 0/4] Driver for talking to PLX PEX8xxx PCIe switch over I2C

[Rajat Jain]

Most of the PLX PCI Express switches (and also switches from other vendors such
IDT) provide an I2C based secondary interface to access and program the switch.
This can be used to program the switch in situations where the PCIe interface
may not be suitable, or even available. (For instance, we may need to program
the device in a particular fashion before we even begin enumerating on the PCI,
or to even enable the PCIe interface of the device). More information on
usecases and other details has been provided in the documentation being added
as part of this patchset.

This is a driver that allows talking to the PLX PEX8xxx family of PCIe switches
over its I2C interface. Here are the patch descriptions:

[Patch 1/4] - Adds the PEX8xxx driver that exports API calls to read / write.
[Patch 2/4] - Adds a sysfs interface, to allow access to the switch.
[Patch 3/4] - Adds the MAINTAINERS entry.
[Patch 4/4] - Adds the documentation in Documentation/PCI/pex8xxx_i2c.txt

Since this is a driver for a PCIe switch, I currently put it under /driver/pci/.
I'm very open to suggestions for moving this around (Does introducing
drivers/pci/switch/ seem any better?). Especially I can see that the
drivers/pci/Kconfig appears under "Bus Options" in the main menu which does not
look like the right place for this driver. I am looking for suggestions here.

Looking forward to comments,

Thanks,

Rajat

Rajat Jain (4):
  pci: Add I2C driver for the PLX PEX8xxx PCIe switch
  pci/pex8xxx: Add sysfs interface for userspace access.
  MAINTAINERS: Add new device driver entry for PEX pex8xxx
  Documentation: Add documentation for the PCI switch PEX8xxx I2C
    driver

 Documentation/PCI/pex8xxx_i2c.txt |  134 +++++++++
 MAINTAINERS                       |    7 +
 drivers/pci/Kconfig               |   13 +
 drivers/pci/Makefile              |    2 +
 drivers/pci/pex8xxx_i2c.c         |  538 +++++++++++++++++++++++++++++++++++++
 include/linux/i2c/pex8xxx_i2c.h   |   36 +++
 6 files changed, 730 insertions(+)
 create mode 100644 Documentation/PCI/pex8xxx_i2c.txt
 create mode 100644 drivers/pci/pex8xxx_i2c.c
 create mode 100644 include/linux/i2c/pex8xxx_i2c.h

-- 
1.7.9.5

Re: [PATCH 0/4] Driver for talking to PLX PEX8xxx PCIe switch over I2C

[Bjorn Helgaas]

[+cc Wolfram]

On Mon, Sep 29, 2014 at 10:46 PM, Rajat Jain <rajatxjain@gmail.com> wrote:
> Most of the PLX PCI Express switches (and also switches from other vendors such
> IDT) provide an I2C based secondary interface to access and program the switch.
> This can be used to program the switch in situations where the PCIe interface
> may not be suitable, or even available. (For instance, we may need to program
> the device in a particular fashion before we even begin enumerating on the PCI,
> or to even enable the PCIe interface of the device). More information on
> usecases and other details has been provided in the documentation being added
> as part of this patchset.
>
> This is a driver that allows talking to the PLX PEX8xxx family of PCIe switches
> over its I2C interface. Here are the patch descriptions:
>
> [Patch 1/4] - Adds the PEX8xxx driver that exports API calls to read / write.
> [Patch 2/4] - Adds a sysfs interface, to allow access to the switch.
> [Patch 3/4] - Adds the MAINTAINERS entry.
> [Patch 4/4] - Adds the documentation in Documentation/PCI/pex8xxx_i2c.txt
>
> Since this is a driver for a PCIe switch, I currently put it under /driver/pci/.
> I'm very open to suggestions for moving this around (Does introducing
> drivers/pci/switch/ seem any better?). Especially I can see that the
> drivers/pci/Kconfig appears under "Bus Options" in the main menu which does not
> look like the right place for this driver. I am looking for suggestions here.

Definitely interesting work.  I have no idea where to put it.  It
doesn't have anything to do with PCI core services or structures, so
drivers/pci doesn't seem like the ideal place for it, but I don't have
any better ideas.  If it did end up under drivers/pci, I agree that a
drivers/pci/switch or something similar would probably be better than
having it directly in drivers/pci.

Maybe Wolfram or other I2C folks will have an idea.

> Rajat Jain (4):
>   pci: Add I2C driver for the PLX PEX8xxx PCIe switch
>   pci/pex8xxx: Add sysfs interface for userspace access.
>   MAINTAINERS: Add new device driver entry for PEX pex8xxx
>   Documentation: Add documentation for the PCI switch PEX8xxx I2C
>     driver
>
>  Documentation/PCI/pex8xxx_i2c.txt |  134 +++++++++
>  MAINTAINERS                       |    7 +
>  drivers/pci/Kconfig               |   13 +
>  drivers/pci/Makefile              |    2 +
>  drivers/pci/pex8xxx_i2c.c         |  538 +++++++++++++++++++++++++++++++++++++
>  include/linux/i2c/pex8xxx_i2c.h   |   36 +++
>  6 files changed, 730 insertions(+)
>  create mode 100644 Documentation/PCI/pex8xxx_i2c.txt
>  create mode 100644 drivers/pci/pex8xxx_i2c.c
>  create mode 100644 include/linux/i2c/pex8xxx_i2c.h
>
> --
> 1.7.9.5
>

Re: [PATCH 0/4] Driver for talking to PLX PEX8xxx PCIe switch over I2C

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1021 bytes --]


> > Since this is a driver for a PCIe switch, I currently put it under /driver/pci/.
> > I'm very open to suggestions for moving this around (Does introducing
> > drivers/pci/switch/ seem any better?). Especially I can see that the
> > drivers/pci/Kconfig appears under "Bus Options" in the main menu which does not
> > look like the right place for this driver. I am looking for suggestions here.
> 
> Definitely interesting work.  I have no idea where to put it.  It
> doesn't have anything to do with PCI core services or structures, so
> drivers/pci doesn't seem like the ideal place for it, but I don't have
> any better ideas.  If it did end up under drivers/pci, I agree that a
> drivers/pci/switch or something similar would probably be better than
> having it directly in drivers/pci.
> 
> Maybe Wolfram or other I2C folks will have an idea.

Hmm, is this a common interface for those switches? Then a directory
like drivers/pci/switch could make sense. Otherwise I'd suggest
drivers/misc?


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 0/4] Driver for talking to PLX PEX8xxx PCIe switch over I2C

[Guenter Roeck]

On Tue, Sep 30, 2014 at 11:13:39PM +0200, Wolfram Sang wrote:
> 
> > > Since this is a driver for a PCIe switch, I currently put it under /driver/pci/.
> > > I'm very open to suggestions for moving this around (Does introducing
> > > drivers/pci/switch/ seem any better?). Especially I can see that the
> > > drivers/pci/Kconfig appears under "Bus Options" in the main menu which does not
> > > look like the right place for this driver. I am looking for suggestions here.
> > 
> > Definitely interesting work.  I have no idea where to put it.  It
> > doesn't have anything to do with PCI core services or structures, so
> > drivers/pci doesn't seem like the ideal place for it, but I don't have
> > any better ideas.  If it did end up under drivers/pci, I agree that a
> > drivers/pci/switch or something similar would probably be better than
> > having it directly in drivers/pci.
> > 
> > Maybe Wolfram or other I2C folks will have an idea.
> 
> Hmm, is this a common interface for those switches? Then a directory
> like drivers/pci/switch could make sense. Otherwise I'd suggest
> drivers/misc?
> 
We had discussed drivers/misc internally, but it didn't seem to be the right
location to me. After all, it is not a misc driver, but a driver to configure
a PCIe switch. Not that I have a better idea, though.

Guenter

Re: [PATCH 0/4] Driver for talking to PLX PEX8xxx PCIe switch over I2C

[Rajat Jain]

Hello,

On Tue, Sep 30, 2014 at 2:13 PM, Wolfram Sang <wsa@the-dreams.de> wrote:
>
>> > Since this is a driver for a PCIe switch, I currently put it under /driver/pci/.
>> > I'm very open to suggestions for moving this around (Does introducing
>> > drivers/pci/switch/ seem any better?). Especially I can see that the
>> > drivers/pci/Kconfig appears under "Bus Options" in the main menu which does not
>> > look like the right place for this driver. I am looking for suggestions here.
>>
>> Definitely interesting work.  I have no idea where to put it.  It
>> doesn't have anything to do with PCI core services or structures, so
>> drivers/pci doesn't seem like the ideal place for it, but I don't have
>> any better ideas.  If it did end up under drivers/pci, I agree that a
>> drivers/pci/switch or something similar would probably be better than
>> having it directly in drivers/pci.
>>
>> Maybe Wolfram or other I2C folks will have an idea.
>
> Hmm, is this a common interface for those switches? Then a directory
> like drivers/pci/switch could make sense. Otherwise I'd suggest
> drivers/misc?
>

Yes, this (I2C) is a common (secondary) interface for most of the
modern PCIe switches from PLX and IDT. And it is often used in complex
systems to handle use cases described in the documentation being added
as part of this patch set. Excerpt (from
https://lkml.org/lkml/2014/9/30/29):

===========================================================
...<snip>...

+0. Why have an I2C interface to a PCIe switch?
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+Other than the regular PCI express interface, most modern PCIe switches (e.g.
+from IDT and PLX) have an I2C based secondary interface. This interface allows
+access to all the registers of the switch. Some of these registers may not even
+be accessible over the regular PCI interface. Also, there are certain registers
+that can be written to, using only the I2C interface and may only be read-only
+using the PCI interface.
+
+This I2C interface is often used in designs involving these switches, and can
+be used for a variety of use cases where the switch needs to be configured
+independent of the PCI subsystem (and likely before PCI enumeration). Some
+examples:
+
+* Dividing a PCIe switch into multiple "virtual" switches. Using this feature,
+  a switch could be connected to 2 root ports for instance, each managing its
+  own PCI hierarchy, and the traffic from one virtual switch does not leak into
+  another.
+
+* Managing Transparent / Non-transparent bridging, and changing them
on-the-fly.
+  There are ports that can be converted into "Non-transparent" bridge ports.
+  Essentially this is used to create different domains (not visible to
+  software). In a dynamic distributed system, it may be desirable to change a
+  transparent bridge to non-transparent or vice versa, for example, to handle a
+  failover situation.
+
+* Buggy hardware / Bad EEPROM configuration. There may be cases where an errata
+  involving register writes need to be applied before enumerating over PCI.
+  Also these switches are typically attached to an EEPROM that is supposed to
+  initialize the switch. If that EEPROM is not present, or contains bad
+  initialization data, this I2C interface can be used to fix that.
+
+* Changing switch configuration on the fly. In a multi-homed or complex
+  distributed systemsystem, there may be a need to change the switch
+  configuration (eg. change the upstream port, or the port or lane
+  configuration etc) to address run time scenarios (CPU plug out etc).
+

...<snip>...
===========================================================

I am myself leaning towards drivers/pci/switch/. And am wanting to
hear what other think.

Thanks,

Rajat

Re: [PATCH 0/4] Driver for talking to PLX PEX8xxx PCIe switch over I2C

[Greg Kroah-Hartman]

On Tue, Sep 30, 2014 at 02:35:51PM -0700, Guenter Roeck wrote:
> On Tue, Sep 30, 2014 at 11:13:39PM +0200, Wolfram Sang wrote:
> > 
> > > > Since this is a driver for a PCIe switch, I currently put it under /driver/pci/.
> > > > I'm very open to suggestions for moving this around (Does introducing
> > > > drivers/pci/switch/ seem any better?). Especially I can see that the
> > > > drivers/pci/Kconfig appears under "Bus Options" in the main menu which does not
> > > > look like the right place for this driver. I am looking for suggestions here.
> > > 
> > > Definitely interesting work.  I have no idea where to put it.  It
> > > doesn't have anything to do with PCI core services or structures, so
> > > drivers/pci doesn't seem like the ideal place for it, but I don't have
> > > any better ideas.  If it did end up under drivers/pci, I agree that a
> > > drivers/pci/switch or something similar would probably be better than
> > > having it directly in drivers/pci.
> > > 
> > > Maybe Wolfram or other I2C folks will have an idea.
> > 
> > Hmm, is this a common interface for those switches? Then a directory
> > like drivers/pci/switch could make sense. Otherwise I'd suggest
> > drivers/misc?
> > 
> We had discussed drivers/misc internally, but it didn't seem to be the right
> location to me. After all, it is not a misc driver, but a driver to configure
> a PCIe switch. Not that I have a better idea, though.

drivers/misc is fine with me, I think we already have something like
this for I2C USB devices somewhere in the tree in that location.

thanks,

greg k-h

Re: [PATCH 0/4] Driver for talking to PLX PEX8xxx PCIe switch over I2C

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 3452 bytes --]


> ===========================================================
> ...<snip>...
> 
> +0. Why have an I2C interface to a PCIe switch?
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +Other than the regular PCI express interface, most modern PCIe switches (e.g.
> +from IDT and PLX) have an I2C based secondary interface. This interface allows
> +access to all the registers of the switch. Some of these registers may not even
> +be accessible over the regular PCI interface. Also, there are certain registers
> +that can be written to, using only the I2C interface and may only be read-only
> +using the PCI interface.
> +
> +This I2C interface is often used in designs involving these switches, and can
> +be used for a variety of use cases where the switch needs to be configured
> +independent of the PCI subsystem (and likely before PCI enumeration). Some
> +examples:
> +
> +* Dividing a PCIe switch into multiple "virtual" switches. Using this feature,
> +  a switch could be connected to 2 root ports for instance, each managing its
> +  own PCI hierarchy, and the traffic from one virtual switch does not leak into
> +  another.
> +
> +* Managing Transparent / Non-transparent bridging, and changing them
> on-the-fly.
> +  There are ports that can be converted into "Non-transparent" bridge ports.
> +  Essentially this is used to create different domains (not visible to
> +  software). In a dynamic distributed system, it may be desirable to change a
> +  transparent bridge to non-transparent or vice versa, for example, to handle a
> +  failover situation.
> +
> +* Buggy hardware / Bad EEPROM configuration. There may be cases where an errata
> +  involving register writes need to be applied before enumerating over PCI.
> +  Also these switches are typically attached to an EEPROM that is supposed to
> +  initialize the switch. If that EEPROM is not present, or contains bad
> +  initialization data, this I2C interface can be used to fix that.
> +
> +* Changing switch configuration on the fly. In a multi-homed or complex
> +  distributed systemsystem, there may be a need to change the switch
> +  configuration (eg. change the upstream port, or the port or lane
> +  configuration etc) to address run time scenarios (CPU plug out etc).
> +
> 
> ...<snip>...
> ===========================================================
> 
> I am myself leaning towards drivers/pci/switch/. And am wanting to
> hear what other think.

I had a closer look and my first question is now if we really need a
kernel driver for this?

The sysfs interface is basically to PEEK/POKE registers of the switch.
How registers get accessed is part of userspace. Well, then it might
even be easier to construct the proper i2c messages in userspace, too,
and send them via i2c-dev (instead of writing to various files in
sysfs).

The Kernel API is also PEEK/POKE register. I am not an PCI expert, but I
can't see a user in this form. What I'd expect would be more like

pci_switch_set_mode(some_device, PCI_BRIDGE_MODE_TRANSPARENT);

and then a shim layer would make sure the proper driver for the switch
sends the proper commands. Again, I don't know if the decision of
setting a mode/creating virtual switches... is even desirable in kernel
space. If so, then this is probably a sub-subsystem
drivers/pci/switches, but needs way more abstraction. Otherwise,
userspace will currently do IMO.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 0/4] Driver for talking to PLX PEX8xxx PCIe switch over I2C

[Rajat Jain]

Hello,

On Wed, Oct 1, 2014 at 12:29 AM, Wolfram Sang <wsa@the-dreams.de> wrote:
>
>> ===========================================================
>> ...<snip>...
>>
>> +0. Why have an I2C interface to a PCIe switch?
>> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> +Other than the regular PCI express interface, most modern PCIe switches (e.g.
>> +from IDT and PLX) have an I2C based secondary interface. This interface allows
>> +access to all the registers of the switch. Some of these registers may not even
>> +be accessible over the regular PCI interface. Also, there are certain registers
>> +that can be written to, using only the I2C interface and may only be read-only
>> +using the PCI interface.
>> +
>> +This I2C interface is often used in designs involving these switches, and can
>> +be used for a variety of use cases where the switch needs to be configured
>> +independent of the PCI subsystem (and likely before PCI enumeration). Some
>> +examples:
>> +
>> +* Dividing a PCIe switch into multiple "virtual" switches. Using this feature,
>> +  a switch could be connected to 2 root ports for instance, each managing its
>> +  own PCI hierarchy, and the traffic from one virtual switch does not leak into
>> +  another.
>> +
>> +* Managing Transparent / Non-transparent bridging, and changing them
>> on-the-fly.
>> +  There are ports that can be converted into "Non-transparent" bridge ports.
>> +  Essentially this is used to create different domains (not visible to
>> +  software). In a dynamic distributed system, it may be desirable to change a
>> +  transparent bridge to non-transparent or vice versa, for example, to handle a
>> +  failover situation.
>> +
>> +* Buggy hardware / Bad EEPROM configuration. There may be cases where an errata
>> +  involving register writes need to be applied before enumerating over PCI.
>> +  Also these switches are typically attached to an EEPROM that is supposed to
>> +  initialize the switch. If that EEPROM is not present, or contains bad
>> +  initialization data, this I2C interface can be used to fix that.
>> +
>> +* Changing switch configuration on the fly. In a multi-homed or complex
>> +  distributed systemsystem, there may be a need to change the switch
>> +  configuration (eg. change the upstream port, or the port or lane
>> +  configuration etc) to address run time scenarios (CPU plug out etc).
>> +
>>
>> ...<snip>...
>> ===========================================================
>>
>> I am myself leaning towards drivers/pci/switch/. And am wanting to
>> hear what other think.
>
> I had a closer look and my first question is now if we really need a
> kernel driver for this?
>
> The sysfs interface is basically to PEEK/POKE registers of the switch.
> How registers get accessed is part of userspace. Well, then it might
> even be easier to construct the proper i2c messages in userspace, too,
> and send them via i2c-dev (instead of writing to various files in
> sysfs).
>
> The Kernel API is also PEEK/POKE register. I am not an PCI expert, but I
> can't see a user in this form. What I'd expect would be more like
>
> pci_switch_set_mode(some_device, PCI_BRIDGE_MODE_TRANSPARENT);

I understand and agree. In fact in the internal version of this driver
(that I have not yet sent out for review), we do have APIs added
similar to what you mention above. Currently I have APIs that:
- Enable / Disable PCIe links.
- Change the upstream port.
- Enable / Disable Non-transparent mode etc.

However, I did not send them all out for review because I don't have
the hardware to try and test them out on ALL the supported devices
(also would require considerable amount of time). I have tested those
APIs on PEX8713 only, because for e.g.I only have PEX8713 in a HW that
connects to 2 CPUs at the same time.

>
> and then a shim layer would make sure the proper driver for the switch
> sends the proper commands. Again, I don't know if the decision of
> setting a mode/creating virtual switches... is even desirable in kernel
> space. If so, then this is probably a sub-subsystem
> drivers/pci/switches, but needs way more abstraction. Otherwise,
> userspace will currently do IMO.

You are right, what this basic driver abstracts is the I2C interface
(i.e. the command format and response format etc) of the PCIe switch,
and not really the PCIe switch itself (Hence I was calling it PEX8xxx
I2C driver. I think that is still a useful abstraction to have,
because that can be used to build the actual logic code that program
the switch. Yes, I agree that we can have another layer of abstraction
so that we have:

- The Core logic code (that knows "How do we want the switch to behave")
- A PEX8xxx driver (that knows "which registers to program")
- A PEX8xxx I2C driver ("How to program those registers") - this driver.

I do understand that your suggestion is to include and bundle the
latter two into this same driver. However since the possibilities
(about which APIs to provide) are too much and not enough hardware to
test it, would it be acceptable if I include those APIs, but support
them for only 1 device (and return error for others)? Note that even
with those APIs, I feel exposing the Read/Write APIs will be useful -
because what core logic wants to achieve can be highly device and
platform specific. Also, a sysfs interface for this switch is proving
to be a very helpful development aid :-) (personal experience :-))

Thanks & Best Regards,

Rajat Jain

Re: [PATCH 0/4] Driver for talking to PLX PEX8xxx PCIe switch over I2C

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 2886 bytes --]


> I understand and agree. In fact in the internal version of this driver
> (that I have not yet sent out for review), we do have APIs added
> similar to what you mention above. Currently I have APIs that:
> - Enable / Disable PCIe links.
> - Change the upstream port.
> - Enable / Disable Non-transparent mode etc.

Now, that sounds better to me...

> However, I did not send them all out for review because I don't have
> the hardware to try and test them out on ALL the supported devices
> (also would require considerable amount of time). I have tested those
> APIs on PEX8713 only, because for e.g.I only have PEX8713 in a HW that
> connects to 2 CPUs at the same time.

That is a common problem to not have enough hardware to test. You could
ask on the PCI mailing list for testers. The solution usually lies in
showing the code rather than not showing the code.

> the switch. Yes, I agree that we can have another layer of abstraction
> so that we have:
> 
> - The Core logic code (that knows "How do we want the switch to behave")
> - A PEX8xxx driver (that knows "which registers to program")
> - A PEX8xxx I2C driver ("How to program those registers") - this driver.
> 
> I do understand that your suggestion is to include and bundle the
> latter two into this same driver.

It definately should be this way. Nobody else than the PEX8xxx driver
should be able to send I2C messeges to the device! And this is
absolutely standard, the logic how to talk to a device knows also how to
prepare the I2C messages. One reason where it could be factored out is
if there are multiple ways of transportation possible, like I2C or SPI.

> However since the possibilities
> (about which APIs to provide) are too much and not enough hardware to
> test it, would it be acceptable if I include those APIs, but support
> them for only 1 device (and return error for others)?

Start with what YOU need and show us (all of it). From there, we can
decide: do we start with one driver and factor out later, do we start
with a sub-subsystem right away, etc... And there is still the question
what APIs you provide, how they are implemented and if we really should
have them in-kernel. I think that question will be more interesting for
Bjorn because I don't really know much about switches in the PCI world.

> with those APIs, I feel exposing the Read/Write APIs will be useful -
> because what core logic wants to achieve can be highly device and
> platform specific.

That could also be solved by fixup-callbacks, but we'd need to see the
core to tell, really.

> Also, a sysfs interface for this switch is proving
> to be a very helpful development aid :-) (personal experience :-))

Sure, but such development aids don't need to be upstream. Especially if
they create ABI such as sysfs-entries.

Thanks and regards,

   Wolfram

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 0/4] Driver for talking to PLX PEX8xxx PCIe switch over I2C

[Rajat Jain]

Hi Wolfram,

Thanks a lot for your review and comments. I understand and will
update with those APIs and send out a V2.

Thanks & Best Regards,

Rajat Jain

On Wed, Oct 1, 2014 at 11:34 PM, Wolfram Sang <wsa@the-dreams.de> wrote:
>
>> I understand and agree. In fact in the internal version of this driver
>> (that I have not yet sent out for review), we do have APIs added
>> similar to what you mention above. Currently I have APIs that:
>> - Enable / Disable PCIe links.
>> - Change the upstream port.
>> - Enable / Disable Non-transparent mode etc.
>
> Now, that sounds better to me...
>
>> However, I did not send them all out for review because I don't have
>> the hardware to try and test them out on ALL the supported devices
>> (also would require considerable amount of time). I have tested those
>> APIs on PEX8713 only, because for e.g.I only have PEX8713 in a HW that
>> connects to 2 CPUs at the same time.
>
> That is a common problem to not have enough hardware to test. You could
> ask on the PCI mailing list for testers. The solution usually lies in
> showing the code rather than not showing the code.
>
>> the switch. Yes, I agree that we can have another layer of abstraction
>> so that we have:
>>
>> - The Core logic code (that knows "How do we want the switch to behave")
>> - A PEX8xxx driver (that knows "which registers to program")
>> - A PEX8xxx I2C driver ("How to program those registers") - this driver.
>>
>> I do understand that your suggestion is to include and bundle the
>> latter two into this same driver.
>
> It definately should be this way. Nobody else than the PEX8xxx driver
> should be able to send I2C messeges to the device! And this is
> absolutely standard, the logic how to talk to a device knows also how to
> prepare the I2C messages. One reason where it could be factored out is
> if there are multiple ways of transportation possible, like I2C or SPI.
>
>> However since the possibilities
>> (about which APIs to provide) are too much and not enough hardware to
>> test it, would it be acceptable if I include those APIs, but support
>> them for only 1 device (and return error for others)?
>
> Start with what YOU need and show us (all of it). From there, we can
> decide: do we start with one driver and factor out later, do we start
> with a sub-subsystem right away, etc... And there is still the question
> what APIs you provide, how they are implemented and if we really should
> have them in-kernel. I think that question will be more interesting for
> Bjorn because I don't really know much about switches in the PCI world.
>
>> with those APIs, I feel exposing the Read/Write APIs will be useful -
>> because what core logic wants to achieve can be highly device and
>> platform specific.
>
> That could also be solved by fixup-callbacks, but we'd need to see the
> core to tell, really.
>
>> Also, a sysfs interface for this switch is proving
>> to be a very helpful development aid :-) (personal experience :-))
>
> Sure, but such development aids don't need to be upstream. Especially if
> they create ABI such as sysfs-entries.
>
> Thanks and regards,
>
>    Wolfram