| Message ID | 20190603230604.30938-1-linus.walleij@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | RFC: fmc: Try to convert to GPIO descriptors | expand |
Hello Linus, I take this occasion to renovate my proposal https://lkml.org/lkml/2018/10/29/534 On Tuesday, June 4, 2019 1:06:04 AM CEST Linus Walleij wrote: > This tries to convert the FMC subsystem to use GPIO descriptors. > I say try because several pieces of this puzzle seems to not > be in the mainline kernel. > > For details of this change, see drivers/gpio/TODO. > > We assume the FMC device is created somewhere and the GPIOs can > be added in this place using gpio descriptor tables from > <linux/gpio/machine.h> as in other conversions, but the place > where the FMC device is created does not seem to be in the > mainline Linux kernel. We assume the index ordering can be > made to match the current GPIO index order in FMC which > isn't especially specific. > > We get rid of some GPIO compatibility defines for kernel 3.0 > in the process, it is long overdue. > > It might be that gpio numbers in this subsystem has nothing > to do with gpio numbers in the GPIO subsystem, and it is just > including <linux/gpio.h> and reusing some defines for no > good reason. Then this should be changed to some approach > decoupling FMC from the GPIO subsystem. > > Cc: Federico Vaga <federico.vaga@cern.ch> > Cc: Pat Riehecky <riehecky@fnal.gov> > Cc: Alessandro Rubini <rubini@gnudd.com> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> > --- > drivers/fmc/fmc-core.c | 1 + > drivers/fmc/fmc-trivial.c | 21 ++++++++++++++++----- > include/linux/fmc.h | 15 ++++----------- > 3 files changed, 21 insertions(+), 16 deletions(-) > > diff --git a/drivers/fmc/fmc-core.c b/drivers/fmc/fmc-core.c > index bbcb505d1522..f08b6f81f442 100644 > --- a/drivers/fmc/fmc-core.c > +++ b/drivers/fmc/fmc-core.c > @@ -14,6 +14,7 @@ > #include <linux/device.h> > #include <linux/fmc.h> > #include <linux/fmc-sdb.h> > +#include <linux/gpio/consumer.h> > > #include "fmc-private.h" > > diff --git a/drivers/fmc/fmc-trivial.c b/drivers/fmc/fmc-trivial.c > index 8defdee3e3a3..bab02d17f02c 100644 > --- a/drivers/fmc/fmc-trivial.c > +++ b/drivers/fmc/fmc-trivial.c > @@ -15,7 +15,7 @@ > #include <linux/module.h> > #include <linux/init.h> > #include <linux/interrupt.h> > -#include <linux/gpio.h> > +#include <linux/gpio/consumer.h> > #include <linux/fmc.h> > > static struct fmc_driver t_drv; /* initialized later */ > @@ -31,12 +31,12 @@ static irqreturn_t t_handler(int irq, void *dev_id) > > static struct fmc_gpio t_gpio[] = { > { > - .gpio = FMC_GPIO_IRQ(0), > - .mode = GPIOF_DIR_IN, > + .gpio_index = FMC_GPIO_IRQ(0), > + .flags = GPIOD_IN, > .irqmode = IRQF_TRIGGER_RISING, > }, { > - .gpio = FMC_GPIO_IRQ(1), > - .mode = GPIOF_DIR_IN, > + .gpio_index = FMC_GPIO_IRQ(1), > + .flags = GPIOD_IN, > .irqmode = IRQF_TRIGGER_RISING, > } > }; > @@ -45,6 +45,7 @@ static int t_probe(struct fmc_device *fmc) > { > int ret; > int index = 0; > + int i; > > index = fmc_validate(fmc, &t_drv); > if (index < 0) > @@ -53,6 +54,16 @@ static int t_probe(struct fmc_device *fmc) > ret = fmc_irq_request(fmc, t_handler, "fmc-trivial", IRQF_SHARED); > if (ret < 0) > return ret; > + /* > + * All GPIOs are associated with the FMC device using machine > + * descriptor tables or similar. Pick the indices we want. > + */ > + for (i = 0; i < ARRAY_SIZE(t_gpio); i++) { > + t_gpio[i].gpiod = devm_gpiod_get_index(&fmc->dev, > + NULL, > + t_gpio[i].gpio_index, > + t_gpio[i].flags); > + } > /* ignore error code of call below, we really don't care */ > fmc_gpio_config(fmc, t_gpio, ARRAY_SIZE(t_gpio)); > > diff --git a/include/linux/fmc.h b/include/linux/fmc.h > index 3dc8a1b2db7b..452688aa97db 100644 > --- a/include/linux/fmc.h > +++ b/include/linux/fmc.h > @@ -15,6 +15,7 @@ > #include <linux/list.h> > #include <linux/interrupt.h> > #include <linux/io.h> > +#include <linux/gpio/consumer.h> > > struct fmc_device; > struct fmc_driver; > @@ -99,9 +100,9 @@ struct fmc_driver { > */ > struct fmc_gpio { > char *carrier_name; /* name or NULL for virtual pins */ > - int gpio; > - int _gpio; /* internal use by the carrier */ > - int mode; /* GPIOF_DIR_OUT etc, from <linux/gpio.h> */ > + int gpio_index; > + struct gpio_desc *gpiod; > + enum gpiod_flags flags; /* see <linux/gpio/consumer.h> */ > int irqmode; /* IRQF_TRIGGER_LOW and so on */ > }; > > @@ -115,14 +116,6 @@ struct fmc_gpio { > #define FMC_GPIO_USER(x) ((x) + 0x1400) /* 256 of them */ > /* We may add SCL and SDA, or other roles if the need arises */ > > -/* GPIOF_DIR_IN etc are missing before 3.0. copy from <linux/gpio.h> */ > -#ifndef GPIOF_DIR_IN > -# define GPIOF_DIR_OUT (0 << 0) > -# define GPIOF_DIR_IN (1 << 0) > -# define GPIOF_INIT_LOW (0 << 1) > -# define GPIOF_INIT_HIGH (1 << 1) > -#endif > - > /* > * The operations are offered by each carrier and should make driver > * design completely independent of the carrier. Named GPIO pins may be
On 04.06.19 06:42, Federico Vaga wrote: > Hello Linus, > > I take this occasion to renovate my proposal > > https://lkml.org/lkml/2018/10/29/534 Is CERN open to invest resources into it (eg. manpower or sponsoring) ? Just a few questions on your original proposal: >> The FMC Standard >> ---------------- >> >> The FMC standard (`VITA-57.1`_) describes FMC modules and carriers >> from the mechanical and the electrical point of view. `VITA-57.1`_ >> also specifies the information stored in the (mandatory) EEPROM on >> the FMC module. So, this is some special backplane bus ? Does the host (linux machine) also act as a module or more like a pci host bridge ? In the latter case it IMHO smells like it deservces its own bus type (which also cares about things like probing logic) >> On the other hand, `VITA-57.1`_ does not specify at all how FMC >> carriers and its FMC modules communicate. <snip> >> 5.5 I2C bus signal >> This chapter specifies that an I2C bus connecting an FMC carrier >> and its FMC modules is mandatory. Consequently, it specifies the FMC >> signals that are reserved for that purpose. >> >> By this chapter, an FMC module must contain an EEPROM connected to >> that >> I2C bus. The purpose of this EEPROM is to store >> *hardware definition information* using the >> `Platform Management FRU Information Storage Definition V1.0`_ Does the fmc carrier have an i2c controller, which in turn the host cpu can speak with ? Is that one directly accessible on the CPUs mem/io bus or do we need some extra transport layer (regmap) ? > So, from a software point of view, we are interested in reading the FRU > > information that provides useful data to identify the FMC modules, and > to do so at a standardized I2C address and EEPROM programming interface. Does that eeprom contain just some model ID (similar to eg. PCI IDs) ? Or maybe something more detailed, similar to oftree or acpi ? > 5.6 Geographical address > This chapter describes how to address the different I2C devices hosted > in the FMC mezzanines that live in a single FMC carrier. What are "FMC mezzanines" ? Specific combinations of cards ? Do they form some kind of composite device ? > As the > standard specifies up to four mezzanines per I2C bus, it enforces the > convention that the two least significant bits of the I2C address > identify the slot. > For example, the mandatory I2C EEPROM must be addressable at the 7-bit > address b10100XX, the suffix XX=00,01,10,11 determining the geographical > addressing of the module in the carrier. Other I2C devices follow the same > schema. Is that information ("card X is in slot Y", correct ?) important for the drivers or even userland ? > The main wrong assumption in the Linux fmc-bus is the idea that > drivers > are bound to FMC modules through the carrier FPGA: this is not true. hmm, smells a bit similar to having a generic FPGA card in PCI, where we can probe the card model itself, but yet have to load a bitfile to do something useful with it. correct ? Maybe introduce an own bus_type - similar to eg. pci, let the carriers be host adapters and the module bus devices. Then it should be pretty much the same scenario like fpga's on pci cards. IMHO. > Behind this lies another wrong assumption: that FMC modules are in a > one-to-one relationship with the FPGA code: this is wrong. The actual > relationship is one-to-many: one FMC module can be driven by many FPGA > code designs, which might be wildly varying. The real behavior of a > card (FMC carrier + FMC module[s]) is determined by the FPGA code that > has been loaded on the FPGA. Is there a way to dynamically identify the currently loaded bitfiles ? Can the fpga's be directly memory mapped to cpu ? My favourite approach would be putting an oftree into the the fpga, on some fixed address. Then the individual module could also be treated as bus adapters, which probe the in-bitfile-devices as platform devices via oftee. > We can have an FMC module with a simple 5 digital I/O channel. We all > know that such module is versatile, so we can have FPGA code that > implements: two I2C masters, an SPI master, a GPIO interface, a TDC, > custom logic. In all these cases the FMC module is the same but the > FPGA code that drives it is different, > as well as the Linux driver that should drive it. hmm, so the module driver somehow has to do the probing, after the actual bitfile is loaded. > So, the fmc-bus design is wrong in trying to match devices and drivers > using > the FRU information from the FMC module EEPROM. IMHO depending on the exact definition of the term "devices and drivers. We'd need to split it into two layers - one for the hardware module, another one for the payloads. I believe the approach I've mentioned above are a good way to do it. On a highlevel view this scenario shouldn't be so different from having an USB host on a PCI card and arbitrarily replacable usb devices behind that. But here replacing devices is done by software (bitfile upload). > FMC Device Class > ---------------- > I propose to retain all the functionality of the fmc-bus that should > remain (see below) in a kernel module implementing an FMC device class > that registers the presence of individual FMC modules. I believe it should actually be a fmc bus_type, so we can benefit from lots of code for the probing / (sub)device instantiation. Few years go I've did a similar approach with some strange railways backplane system. The usecase was a little bit less complex (basically, a memmapped fpga that implements a bunch of proprietary serial pipes and the cards behind them). > FPGA Manager > The Linux fmc-bus asks the FMC carrier driver to export the necessary > operations to program the FMC carrier FPGA. Currently, this is handled > by the Linux kernel FPGA manager subsystem. Can it already trigger some probing ? > So, the FMC carrier driver should create a device instance for the FPGA manager. The bitfile loading is done by the carrier, not the modules ? > I2C Master Driver > The Linux fmc-bus current implementation asks the FMC carrier drivers > to > implement the I2C operations in order to access the EEPROM. In theory > this > is not really necessary; Linux provides a nice I2C sub-system for this > purpose. Only the particular I2C master instance needs to be registered by > the FMC carrier driver. Are these standard i2c controllers ? > EEPROM Driver > The Linux fmc-bus uses the I2C programming interface of an FMC module EEPROM > to extract its contents, in particular the module identification for driver > Again, this is rendered superfluous by the Linux kernel I2C EEPROM > drivers. There is, however, a difficulty with the particular EEPROM > to use: which one is it? `VITA-57.1`_ is unclear about this. > rule 5.69 about what kind of EEPROM is mandatory: Seems plausible. > to support 24C02 EEPROMs. Support of other EEPROMs should not be > excluded, though, as `VITA-57.1`_ is admittedly open > misinterpretaion > on this point. Are there some known cases where a different eeprom is used ? > IRQ Controller > The Linux fmc-bus asks the FMC carrier to implement the operations to > request, free and acknowledge interrupts. This is necessary because > FMC modules plugged into an FMC carrier share the unique interrupt > line or vector of the latter. However, this function does not belong > in the fmc-bus. As the obvious need for such logic suggests, > the FMC carrier contains interrupt routing logic; thus, an IRQ > controller is the proper way to handle this. Agreed. > As stated before, there is no such thing as an "FMC module driver", > nor a "device driver for the FPGA code". Oh, seems I had a wrong idea of that. Do the individual module types differ - from cpu pov - in anyway, despite the bitfile specific interfaces ? If not, what's the actual purpose of the eeprom data ? > So, the only way I see it possible is by using platform drivers for > the *FPGA internal devices*. The description of the internal devices > is not within the scope of the FMC software layer. Who shall be responsible for instantiating these platform devices ? Maybe oftree overlays ? --mtx -- Enrico Weigelt, metux IT consult Free software and Linux embedded engineering info@metux.net -- +49-151-27565287
Hello Enrico, from your questions is clear that you do not know the fmc-bus and neither the VITA-57.1 (FMC) standard or the IPMI FRU specification. Some of your questions are answered by those documents (VITA-57.1, Platform Management FRU Information Storage Definition V1.0, fmc-bus documentation in Documentation/ fmc); of course you want to understand but I think is better if you read those documents first because I do not want to answer to direct questions without having all the background knowledge supporting it (the risk is that out of context they are open to misinterpretation). So, I will focus my answer only on those point which are not covered there. On Tuesday, June 4, 2019 7:24:46 PM CEST Enrico Weigelt, metux IT consult wrote: > On 04.06.19 06:42, Federico Vaga wrote: > > Hello Linus, > > > > I take this occasion to renovate my proposal > > > > https://lkml.org/lkml/2018/10/29/534 > > Is CERN open to invest resources into it (eg. manpower > or sponsoring) ? > > Just a few questions on your original proposal: > >> The FMC Standard > >> ---------------- > >> > >> The FMC standard (`VITA-57.1`_) describes FMC modules and carriers > >> from the mechanical and the electrical point of view. `VITA-57.1`_ > >> also specifies the information stored in the (mandatory) EEPROM on > >> the FMC module. > > So, this is some special backplane bus ? Despite the fact that FMC on Linux is a Linux bus (a mechanism to match devices and driver with some logic), it is not a real bus. Form software prospective it is an I2C EEPROM that identifies an FMC module. A part from this connection with the host, the FMC module is not accessible. All accesses pass through an FPGA which interface is not in a one-to-one relation with any FMC module. VITA-57.1 standardize the connection between an FPGA and an FMC module. In addition it offers this I2C interface to identify the connected FMC module(s). > Does the host (linux machine) also act as a module or more like a pci > host bridge ? In the latter case it IMHO smells like it deservces its > own bus type (which also cares about things like probing logic) > > >> On the other hand, `VITA-57.1`_ does not specify at all how FMC > >> carriers and its FMC modules communicate. > > <snip> > > >> 5.5 I2C bus signal > >> > >> This chapter specifies that an I2C bus connecting an FMC carrier > >> and its FMC modules is mandatory. Consequently, it specifies the FMC > >> signals that are reserved for that purpose. > >> > >> By this chapter, an FMC module must contain an EEPROM connected to > >> > >> that > >> > >> I2C bus. The purpose of this EEPROM is to store > >> *hardware definition information* using the > >> `Platform Management FRU Information Storage Definition V1.0`_ > > Does the fmc carrier have an i2c controller, which in turn the host > cpu can speak with ? Is that one directly accessible on the CPUs > mem/io bus or do we need some extra transport layer (regmap) ? > > > So, from a software point of view, we are interested in reading the FRU > > > > information that provides useful data to identify the FMC modules, and > > to do so at a standardized I2C address and EEPROM programming interface. > > Does that eeprom contain just some model ID (similar to eg. PCI IDs) ? > Or maybe something more detailed, similar to oftree or acpi ? > > > 5.6 Geographical address > > > > This chapter describes how to address the different I2C devices hosted > > in the FMC mezzanines that live in a single FMC carrier. > > What are "FMC mezzanines" ? Specific combinations of cards ? > Do they form some kind of composite device ? > > > As the > > standard specifies up to four mezzanines per I2C bus, it enforces the > > convention that the two least significant bits of the I2C address > > identify the slot. > > For example, the mandatory I2C EEPROM must be addressable at the 7-bit > > address b10100XX, the suffix XX=00,01,10,11 determining the geographical > > addressing of the module in the carrier. Other I2C devices follow > > the same > > > schema. > > Is that information ("card X is in slot Y", correct ?) important for the > drivers or even userland ? This sort of information is always important when you what to be able to distinguish cards when you look at then with your eyes, or when those FMC modules are used to create a network with external equipment > > The main wrong assumption in the Linux fmc-bus is the idea that > > drivers > > are bound to FMC modules through the carrier FPGA: this is not true. > > hmm, smells a bit similar to having a generic FPGA card in PCI, where > we can probe the card model itself, but yet have to load a bitfile to > do something useful with it. correct ? > > Maybe introduce an own bus_type - similar to eg. pci, let the carriers > be host adapters and the module bus devices. Then it should be pretty > much the same scenario like fpga's on pci cards. IMHO. > > > Behind this lies another wrong assumption: that FMC modules are in a > > one-to-one relationship with the FPGA code: this is wrong. The actual > > relationship is one-to-many: one FMC module can be driven by many FPGA > > code designs, which might be wildly varying. The real behavior of a > > card (FMC carrier + FMC module[s]) is determined by the FPGA code that > > has been loaded on the FPGA. > > Is there a way to dynamically identify the currently loaded bitfiles ? > Can the fpga's be directly memory mapped to cpu ? > > My favourite approach would be putting an oftree into the the fpga, on > some fixed address. Then the individual module could also be treated > as bus adapters, which probe the in-bitfile-devices as platform devices > via oftee. > > > We can have an FMC module with a simple 5 digital I/O channel. We all > > know that such module is versatile, so we can have FPGA code that > > implements: two I2C masters, an SPI master, a GPIO interface, a TDC, > > custom logic. In all these cases the FMC module is the same but the > > FPGA code that drives it is different, > > as well as the Linux driver that should drive it. > > hmm, so the module driver somehow has to do the probing, after the > actual bitfile is loaded. > > > So, the fmc-bus design is wrong in trying to match devices and drivers > > using > > the FRU information from the FMC module EEPROM. > > IMHO depending on the exact definition of the term "devices and drivers. > We'd need to split it into two layers - one for the hardware module, > another one for the payloads. I believe the approach I've mentioned > above are a good way to do it. > > On a highlevel view this scenario shouldn't be so different from having > an USB host on a PCI card and arbitrarily replacable usb devices behind > that. But here replacing devices is done by software (bitfile upload). > > > FMC Device Class > > ---------------- > > I propose to retain all the functionality of the fmc-bus that should > > remain (see below) in a kernel module implementing an FMC device class > > that registers the presence of individual FMC modules. > > I believe it should actually be a fmc bus_type, so we can benefit from > lots of code for the probing / (sub)device instantiation. > > Few years go I've did a similar approach with some strange railways > backplane system. The usecase was a little bit less complex (basically, > a memmapped fpga that implements a bunch of proprietary serial pipes > and the cards behind them). > > > FPGA Manager > > > > The Linux fmc-bus asks the FMC carrier driver to export the necessary > > operations to program the FMC carrier FPGA. Currently, this is handled > > by the Linux kernel FPGA manager subsystem. > > Can it already trigger some probing ? > > > So, the FMC carrier driver should create a device instance for the FPGA > > manager. > > The bitfile loading is done by the carrier, not the modules ? > > > I2C Master Driver > > > > The Linux fmc-bus current implementation asks the FMC carrier > > drivers > to > > > implement the I2C operations in order to access the EEPROM. In > > theory > this > > > is not really necessary; Linux provides a nice I2C sub-system for this > > purpose. Only the particular I2C master instance needs to be > > registered by > > > the FMC carrier driver. > > Are these standard i2c controllers ? > > > EEPROM Driver > > > > The Linux fmc-bus uses the I2C programming interface of an FMC > > module EEPROM > > > to extract its contents, in particular the module identification for > > driver > > > Again, this is rendered superfluous by the Linux kernel I2C EEPROM > > drivers. There is, however, a difficulty with the particular EEPROM > > to use: which one is it? `VITA-57.1`_ is unclear about this. > > > rule 5.69 about what kind of EEPROM is mandatory: > Seems plausible. > > > to support 24C02 EEPROMs. Support of other EEPROMs should not be > > excluded, though, as `VITA-57.1`_ is admittedly open > > > > misinterpretaion > > > > on this point. > > Are there some known cases where a different eeprom is used ? For example, all open-hardware designs that my group at CERN is producing, but there are more out there which are "not standard". This happened because, as I wrote and VITA admitted, the standard is open to misinterpretation; VITA promised to fix it on the next revision. > > > IRQ Controller > > > > The Linux fmc-bus asks the FMC carrier to implement the operations to > > request, free and acknowledge interrupts. This is necessary because > > FMC modules plugged into an FMC carrier share the unique interrupt > > line or vector of the latter. However, this function does not belong > > in the fmc-bus. As the obvious need for such logic suggests, > > the FMC carrier contains interrupt routing logic; thus, an IRQ > > controller is the proper way to handle this. > > Agreed. > > > As stated before, there is no such thing as an "FMC module driver", > > nor a "device driver for the FPGA code". > > Oh, seems I had a wrong idea of that. Do the individual module types > differ - from cpu pov - in anyway, despite the bitfile specific > interfaces ? I am not sure to understand your question > If not, what's the actual purpose of the eeprom data ? > > > So, the only way I see it possible is by using platform drivers for > > the *FPGA internal devices*. The description of the internal devices > > is not within the scope of the FMC software layer. > > Who shall be responsible for instantiating these platform devices ? > Maybe oftree overlays ? Could be DeviceTree, ACPI descriptors, or another driver. There are plenty of possibilities. I have noticed that DeviceTree is extensively used in some context. > > --mtx
On 05.06.19 09:55, Federico Vaga wrote: > Hello Enrico, > > from your questions is clear that you do not know the fmc-bus and neither the > VITA-57.1 (FMC) standard or the IPMI FRU specification. Ideed, I never came anywhere near it. (guess, that applies to most people here). So, forgive me for asking some stupid questions. > Some of your questions > are answered by those documents (VITA-57.1, Platform Management FRU > Information Storage Definition V1.0, fmc-bus documentation in Documentation/ > fmc); thanks, I'm just reading the kernel docs (unfortunately lacking the time to work through the whole specs). Let me some more stupid questions: * am I'm correctly assuming that there can be different FMCs, which in turn can be deployed with different application specific bitfiles ? * could the SDB data be dynamically changed at runtime ? does it make sense to expose it as an actual fs to userland ? * is not having the SDB data a common usecase ? * GPIO: are these actually in the carrier itself (not on the cards plugged into it) ? Who's gonna consume them ? (also userland ?) Can we implement this as generic gpio drivers and use gpiod_lookup_table ? * chardev: is some kinda /dev/mem for the fmc devices ? is it used by applications or just development ? * eeprom: should we introduce an actual eeprom device class ? several years ago, somebody was working on that, but don't know what happended it it: https://lore.kernel.org/patchwork/patch/436179/ By the way: looks like the git repos aren't publiclly accessible: (gitlab web frontend seems to work) >> nekrad@orion:~/src/fmc$ git clone -vvvv git://ohwr.org/hdl-core-lib/fpga-config-space.git >> Cloning into 'fpga-config-space'... >> fatal: unable to connect to ohwr.org: >> ohwr.org[0: 188.185.83.37]: errno=Connection timed out >> nekrad@orion:~/src/fmc$ git clone https://ohwr.org/project/fpga-config-space.git >> Cloning into 'fpga-config-space'... >> fatal: unable to access 'https://ohwr.org/project/fpga-config-space.git/': gnutls_handshake() failed: Handshake failed >> So, this is some special backplane bus ? > > Despite the fact that FMC on Linux is a Linux bus (a mechanism to match > devices and driver with some logic), it is not a real bus. Form software > prospective it is an I2C EEPROM that identifies an FMC module. A part from > this connection with the host, the FMC module is not accessible. All accesses > pass through an FPGA which interface is not in a one-to-one relation with any > FMC module. > > VITA-57.1 standardize the connection between an FPGA and an FMC module. In > addition it offers this I2C interface to identify the connected FMC module(s). I guess the connection to the host is very FMC specific. Which IO operations can we do here ? Just register read/write ? Might that be a case for regmap ? I'm imagining giving struct fmc_operations a ref (or get/put) to a regmap instance instead of readl()/writel() operations. That way, subdevice drivers (for some generic logic inside the fpga, eg. a uart, gpio, etc) don't need to be fmc-aware at all - they'd just be initialized by some bitfile-specific parent driver. By the way: is there already name for bitfile specific devices ? (maybe call it "fmc payload" ?) >> Is that information ("card X is in slot Y", correct ?) important for the >> drivers or even userland ? > > This sort of information is always important when you what to be able to > distinguish cards when you look at then with your eyes, or when those FMC > modules are used to create a network with external equipment Aha, so it's something that an operator needs to know, like "this is the first uart" or "that's the 3rd eth port" ? >> Are there some known cases where a different eeprom is used ? > > For example, all open-hardware designs that my group at CERN is producing, but > there are more out there which are "not standard". This happened because, as I > wrote and VITA admitted, the standard is open to misinterpretation; VITA > promised to fix it on the next revision. hmm, is there any way for probing the eeprom type or is that something the operator needs to know/configure ? Not sure whether I really understood the whole, so I'll try to summarize - correct me if I'm wrong: * each FMC carries an i2c eeprom with the SDB data, and potentially extra payload specific data (eg. sdbfs) * there may be different eeprom types (speaking different protocols) * this i2c eeprom is connected to an i2c controller on the carrier * cpu can talk to the carrier in order to do i2c transactions (thus, from cpu pov, the carrier implements an an i2c controller that's connected to that eeprom) * the i2c controller type (more precisely: the cpu-visible protocol) is known by carrier type Are this standard i2c controllers that are already supported by the i2c subsystem ? >> > As stated before, there is no such thing as an "FMC module driver", >> > nor a "device driver for the FPGA code". >> >> Oh, seems I had a wrong idea of that. Do the individual module types >> differ - from cpu pov - in anyway, despite the bitfile specific >> interfaces ? > > I am not sure to understand your question My question is whether the FMCs, besides the current bitfile in FPGA, differ from host point of view. IOW: does the host need to care about the actual FMC type, before we have an actual bitfile loaded ? (maybe fpga specific upload protocols or check whether bitfile fits that FMC ?) >> Who shall be responsible for instantiating these platform devices ? >> Maybe oftree overlays ? > > Could be DeviceTree, ACPI descriptors, or another driver. There are plenty of > possibilities. I have noticed that DeviceTree is extensively used in some > context. So, this part isn't defined yet ? How is that currrently handled ? Could you put a dtb directly in the FPGA, at some defined address ? Some quick idea floating around in my head: we could place a dtb right next to the bitfile (one dtb per bitfile), and when fpga payload is up and running, start dt-based probing of platform devices. hmm, there're several things, I'd like to put on my fun-project-list. Do you happen to have some remote-accesible testbed for playground ? --mtx -- Enrico Weigelt, metux IT consult Free software and Linux embedded engineering info@metux.net -- +49-151-27565287
On Thursday, June 6, 2019 3:58:35 PM CEST Enrico Weigelt, metux IT consult wrote: > On 05.06.19 09:55, Federico Vaga wrote: > > Hello Enrico, > > > > from your questions is clear that you do not know the fmc-bus and neither > > the VITA-57.1 (FMC) standard or the IPMI FRU specification. > > Ideed, I never came anywhere near it. (guess, that applies to most > people here). So, forgive me for asking some stupid questions. > > > Some of your questions > > are answered by those documents (VITA-57.1, Platform Management FRU > > Information Storage Definition V1.0, fmc-bus documentation in > > Documentation/ fmc); > > thanks, I'm just reading the kernel docs (unfortunately lacking the time > to work through the whole specs). > > Let me some more stupid questions: > > * am I'm correctly assuming that there can be different FMCs, which in > turn can be deployed with different application specific bitfiles ? yes > * could the SDB data be dynamically changed at runtime ? > does it make sense to expose it as an actual fs to userland ? > * is not having the SDB data a common usecase ? SDB is something that has been attached to FMC but actually is totally unrelated. There were plans about it in the past but they have been abandon. > * GPIO: are these actually in the carrier itself (not on the cards > plugged into it) ? Who's gonna consume them ? (also userland ?) > Can we implement this as generic gpio drivers and use > gpiod_lookup_table ? There are no relations between GPIO and FMC. GPIO usage is design specific, and in particular specific to the carrier not the modules. > * chardev: is some kinda /dev/mem for the fmc devices ? > is it used by applications or just development ? probably development. At least at CERN nobody uses it; it has been used to hack around at the very beginning > * eeprom: should we introduce an actual eeprom device class ? > several years ago, somebody was working on that, but don't know > what happended it it: > https://lore.kernel.org/patchwork/patch/436179/ I do not think is necessary. The kernel has support for I2C, it has support to the FMC standard EEPROM. They are easy to configure > By the way: looks like the git repos aren't publiclly accessible: > (gitlab web frontend seems to work) > > >> nekrad@orion:~/src/fmc$ git clone -vvvv > > git://ohwr.org/hdl-core-lib/fpga-config-space.git > > >> Cloning into 'fpga-config-space'... > >> fatal: unable to connect to ohwr.org: > >> ohwr.org[0: 188.185.83.37]: errno=Connection timed out > >> > >> nekrad@orion:~/src/fmc$ git clone > > https://ohwr.org/project/fpga-config-space.git > > >> Cloning into 'fpga-config-space'... > >> fatal: unable to access > > 'https://ohwr.org/project/fpga-config-space.git/': gnutls_handshake() > failed: Handshake failed I can report the error, but as you can see that project is abandon. > >> So, this is some special backplane bus ? > > > > Despite the fact that FMC on Linux is a Linux bus (a mechanism to match > > devices and driver with some logic), it is not a real bus. Form software > > prospective it is an I2C EEPROM that identifies an FMC module. A part from > > this connection with the host, the FMC module is not accessible. All > > accesses pass through an FPGA which interface is not in a one-to-one > > relation with any FMC module. > > > > VITA-57.1 standardize the connection between an FPGA and an FMC module. In > > addition it offers this I2C interface to identify the connected FMC > > module(s). > I guess the connection to the host is very FMC specific. Which > IO operations can we do here ? Just register read/write ? It is not FMC specific. FMC is just a connector that you can use to extend a PCI/VME/USB/ISA/SPI/... card > Might that be a case for regmap ? > > I'm imagining giving struct fmc_operations a ref (or get/put) to a > regmap instance instead of readl()/writel() operations. That way, > subdevice drivers (for some generic logic inside the fpga, eg. a uart, > gpio, etc) don't need to be fmc-aware at all - they'd just be > initialized by some bitfile-specific parent driver. > > By the way: is there already name for bitfile specific devices ? > (maybe call it "fmc payload" ?) > > >> Is that information ("card X is in slot Y", correct ?) important for the > >> drivers or even userland ? > > > > This sort of information is always important when you what to be able to > > distinguish cards when you look at then with your eyes, or when those FMC > > modules are used to create a network with external equipment > > Aha, so it's something that an operator needs to know, like "this is > the first uart" or "that's the 3rd eth port" ? Yes > >> Are there some known cases where a different eeprom is used ? > > > > For example, all open-hardware designs that my group at CERN is producing, > > but there are more out there which are "not standard". This happened > > because, as I wrote and VITA admitted, the standard is open to > > misinterpretation; VITA promised to fix it on the next revision. > > hmm, is there any way for probing the eeprom type or is that something > the operator needs to know/configure ? In principle there is only one correct EEPROM: 24C02. Any other eeprom is non- standard, so the operator must know what to do in that specific case. > Not sure whether I really understood the whole, so I'll try to > summarize - correct me if I'm wrong: > > * each FMC carries an i2c eeprom with the SDB data, and potentially > extra payload specific data (eg. sdbfs) No. FMc Modules (plugged on FMC carriers) carry an I2C EEPROM with the "Platform Management FRU Information Storage Definition V1.0." > * there may be different eeprom types (speaking different protocols) According to the standard no. But since the standard is unclear, people misunderstood this point, so it is possible to have different eeproms > * this i2c eeprom is connected to an i2c controller on the carrier Yes > * cpu can talk to the carrier in order to do i2c transactions > (thus, from cpu pov, the carrier implements an an i2c controller > that's connected to that eeprom) yes > * the i2c controller type (more precisely: the cpu-visible protocol) > is known by carrier type yes > Are this standard i2c controllers that are already supported by the > i2c subsystem ? There is not a standard I2C controller. It is a carrier design choice > >> > As stated before, there is no such thing as an "FMC module driver", > >> > nor a "device driver for the FPGA code". > >> > >> Oh, seems I had a wrong idea of that. Do the individual module types > >> differ - from cpu pov - in anyway, despite the bitfile specific > >> interfaces ? > > > > I am not sure to understand your question > > My question is whether the FMCs, besides the current bitfile in FPGA, > differ from host point of view. IOW: does the host need to care about > the actual FMC type, before we have an actual bitfile loaded ? > (maybe fpga specific upload protocols or check whether bitfile fits > that FMC ?) It depends on the context. If your FPGA is empty and you want to load a new configuration then you should check the type of FMC Module plugged in order to avoid to "burn" it (really rare, but it could happen) > >> Who shall be responsible for instantiating these platform devices ? > >> Maybe oftree overlays ? > > > > Could be DeviceTree, ACPI descriptors, or another driver. There are plenty > > of possibilities. I have noticed that DeviceTree is extensively used in > > some context. > > So, this part isn't defined yet ? It does not need to be defined. It is out of scope. How FPGA devices are declare is a general problem, not and FMC problem > How is that currrently handled ? The way fmc-bus is designed, does not need to discover FPGA devices. > Could you put a dtb directly in the FPGA, at some defined address ? In general yes. Why not? > Some quick idea floating around in my head: we could place a dtb > right next to the bitfile (one dtb per bitfile), and when fpga payload > is up and running, start dt-based probing of platform devices. Yes it is a possibility but > hmm, there're several things, I'd like to put on my fun-project-list. Well, I am suggesting to erase fmc-bus from the Linux kernel and to provide a device_class for a standardized way to access that I2C EEPROM. I already have the implementation; perhaps it needs some clean-up and for sure to be ported to 5.x (I will do this job only if there is interest in removing/replacing the current fmc subsystem). I can tell you that it is not that funny :D it is only 484 line of code, most of them to handle the special case where the EEPROM is not-standard. > Do you happen to have some remote-accesible testbed for playground ? Of course not :D CERN infrastructure is not freely accessible > > --mtx
On Tue, Jun 4, 2019 at 8:42 AM Federico Vaga <federico.vaga@cern.ch> wrote: > I take this occasion to renovate my proposal > https://lkml.org/lkml/2018/10/29/534 Hm, it looks from your proposal like the FMC device core is using its own hacks for i2c mastering, FPGA management and irqchips. Does it also have its own GPIO implementation not using struct gpio_chip from <linux/gpio/driver.h>? In that case my first step should rather be to remove the reference to <linux/gpio.h> because then FMC is not really using the GPIO subsystem. As for the proposal so remove it, I'd like to hear what Alessandro says. If there is consensus on deleting this I can queue something up and send it to Torvalds, but I need consent from everyone who cares. Yours, Linus Walleij
On Saturday, June 8, 2019 3:46:07 PM CEST Linus Walleij wrote: > On Tue, Jun 4, 2019 at 8:42 AM Federico Vaga <federico.vaga@cern.ch> wrote: > > I take this occasion to renovate my proposal > > https://lkml.org/lkml/2018/10/29/534 > > Hm, it looks from your proposal like the FMC device core > is using its own hacks for i2c mastering, FPGA management > and irqchips. Does it also have its own GPIO implementation > not using struct gpio_chip from <linux/gpio/driver.h>? The FMC subsystem has its own API for all those things. > In that case my first step should rather be to remove the > reference to <linux/gpio.h> because then FMC is not really > using the GPIO subsystem. No it is not. Probably you can safely remove it. > As for the proposal so remove it, I'd like to hear what > Alessandro says. If there is consensus on deleting this > I can queue something up and send it to Torvalds, but > I need consent from everyone who cares. > > Yours, > Linus Walleij -- Federico Vaga [CERN BE-CO-HT]
> As for the proposal so remove it, I'd like to hear what > Alessandro says. I am not involved in the CERN group any more, so I really have no say: Federico leads, and I agree. I found the fmc-bus a good idea at the time, and we also talked with Lars-Peter Clausen of Analog Devices, who was using fmc hardware and told he would use our framework. We pushed it shortly later, but I Never heard back. I don't know if there are other users, besides the various projects that split off cern open-hardware efforts (e.g., the Zen board by Seven Solutions). Maybe they could just maintain the thing off-tree. thanks /alessandro
On 08.06.19 17:00, Federico Vaga wrote: Hi, >> Hm, it looks from your proposal like the FMC device core >> is using its own hacks for i2c mastering, FPGA management >> and irqchips. Does it also have its own GPIO implementation >> not using struct gpio_chip from <linux/gpio/driver.h>? > > The FMC subsystem has its own API for all those things. Any plans to change that in the forseeable future ? Is userland relying on that own implementation, or could it just use standard gpio subsystem ? --mtx -- Enrico Weigelt, metux IT consult Free software and Linux embedded engineering info@metux.net -- +49-151-27565287
On 10.06.19 08:13, Alessandro Rubini wrote: Hi, > I found the fmc-bus a good idea at the time, and we also talked with> Lars-Peter Clausen of Analog Devices, who was using fmc hardware and> told he would use our framework. We pushed it shortly later, but I> Never heard back. Do you (or @Federico) recall the original design goals ? Leaving aside the purely electrical / mechanical aspects, why not just using either pcix or vme directly ? > I don't know if there are other users, besides the various projects> that split off cern open-hardware efforts (e.g., the Zen board by Seven> Solutions). Maybe they could just maintain the thing off-tree. hmm, OOT drivers are usually ugly to maintain - I'd rather prefer maintaining downstream branches (which are rebased to mainline releases from time to time). --mtx -- Enrico Weigelt, metux IT consult Free software and Linux embedded engineering info@metux.net -- +49-151-27565287
> Do you (or @Federico) recall the original design goals ? Sure. This FMC (fpga mezzanine carrier) is a "slot" for peripheral boards. Like shields for beaglebone or such stuff. So there is a board (and our own are actually pci-x and vme) with the FPGA, and the pins are carried to this "mezzanine" where the I/O happens. The mezzanine is usually just dumb, but it has the mandatory eeprom. So, our "fine delay" board, that can timestamp pulses and generate other pulses at precise times) can plug in both the pcix and vme carrier. The bus abstraction filled nicely in my opinion. The "spec" and "svec" (simple pcix/vme carriers) drivers would register the device, named according to the eeprom contents, and the driver was the same irrespective of the carrier. The drivers for spec and svec (or the mezzanine) were not upstreamed, but the mechanism was, including software-only examples of how to use it, in the hope others would find it useful. Back then there was no generalized way to load fpga binaries, so we had our own, based on firmware loader. Then things went more complex, and federico started to have issues. The tow major ones, IIUC, are (1) the same mezzanine would require different bitstreams according to external information and (2) the fpga binary itself my expose different "devices" where a generalized driver would benefit, instead of a mezzanine-specific driver. > Leaving aside the purely electrical / mechanical aspects, why not > just using either pcix or vme directly ? pcix and vme hosted the carrier. Much like an USB controller can either be PCI or memory-mapped, but the devices it register have a driver that is irrespective of this detail. /alessandro
On 06.06.19 16:41, Federico Vaga wrote: Hi, >> * am I'm correctly assuming that there can be different FMCs, which in >> turn can be deployed with different application specific bitfiles ? > > yes Ok, then - from LDM point of view - I'd prefer treating the FMC as kind of an bus adapter. That "bus" would also employ fpga subsystem for the bitfile deployment (maybe, pure fw-loader would also do ?) and then let it do the logical probing. Sounds a bit cleaner to me than the mfd approach. > SDB is something that has been attached to FMC but actually is totally > unrelated. There were plans about it in the past but they have been abandon. So, we can ignore it ? > There are no relations between GPIO and FMC. GPIO usage is design specific, > and in particular specific to the carrier not the modules. Now I'm confused. If the gpio's are design specific, means they're implemented inside fpga ? But how exactly are they carrier specific ? (by "carrier" you mean the adapter to pcie or vme ?) >> * chardev: is some kinda /dev/mem for the fmc devices ? >> is it used by applications or just development ? > > probably development. At least at CERN nobody uses it; it has been used to > hack around at the very beginning So, not part of the uapi ? (IOW: could we drop it, w/o causing huge pain ?) >> * eeprom: should we introduce an actual eeprom device class ? >> several years ago, somebody was working on that, but don't know >> what happended it it: >> https://lore.kernel.org/patchwork/patch/436179/ > > I do not think is necessary. The kernel has support for I2C, it has support to > the FMC standard EEPROM. They are easy to configure How does the fmc subsystem talk to these drivers ? Just had a quick look on a few of them - these just provide access via sysfs, but haven't found an internal API, where other drivers/subsystems talk to them directly. Is that all done in userland ? >> 'https://ohwr.org/project/fpga-config-space.git/': gnutls_handshake() >> failed: Handshake failed > > I can report the error, but as you can see that project is abandon. hmm, now only used internally ? >> I guess the connection to the host is very FMC specific. Which >> IO operations can we do here ? Just register read/write ? > > It is not FMC specific. FMC is just a connector that you can use to extend a > PCI/VME/USB/ISA/SPI/... card But there're different types (depending on which bus they're plugged into), and these parts have to be type specific ? > In principle there is only one correct EEPROM: 24C02. Any other eeprom is non- > standard, so the operator must know what to do in that specific case. Ok. From where is the eeprom accessed ? Userland or kernel ? >> Not sure whether I really understood the whole, so I'll try to >> summarize - correct me if I'm wrong: >> >> * each FMC carries an i2c eeprom with the SDB data, and potentially >> extra payload specific data (eg. sdbfs) > > No. > FMc Modules (plugged on FMC carriers) carry an I2C EEPROM with the "Platform > Management FRU Information Storage Definition V1.0." Ah, okay, so no sdb data. >> * there may be different eeprom types (speaking different protocols) > > According to the standard no. But since the standard is unclear, people > misunderstood this point, so it is possible to have different eeproms Okay, so we practically have different types (even if that violates the spec). Does that have to be supported ? >> * this i2c eeprom is connected to an i2c controller on the carrier > > Yes So, the carrier forms an MFD which holds an bus adapter as well as an i2c controller. I guess the fmc subsystem needs to parse/handle the eeprom data (for probing, etc), correct ? IMHO, the fmc carrier driver should instanciate an i2c controller and an eeprom behind that. As an intermediate step, I'd imagine changing the built in i2c functions to call into the standard i2c subsystem. >> Are this standard i2c controllers that are already supported by the >> i2c subsystem ? > > There is not a standard I2C controller. It is a carrier design choice Are some of them compatible w/ any already supported i2c controller ? >> My question is whether the FMCs, besides the current bitfile in FPGA, >> differ from host point of view. IOW: does the host need to care about >> the actual FMC type, before we have an actual bitfile loaded ? >> (maybe fpga specific upload protocols or check whether bitfile fits >> that FMC ?) > > It depends on the context. If your FPGA is empty and you want to load a new > configuration then you should check the type of FMC Module plugged in order to > avoid to "burn" it (really rare, but it could happen) Understood. Is this already done automatically, or manually by the operator ? >>> Could be DeviceTree, ACPI descriptors, or another driver. There are plenty >>> of possibilities. I have noticed that DeviceTree is extensively used in >>> some context. >> >> So, this part isn't defined yet ? > > It does not need to be defined. It is out of scope. How FPGA devices are > declare is a general problem, not and FMC problem But somebody needs to start the probing process (however this actually looks like) - finally needs not instanciate the platform devices with the correct configuration. Does subsystem already has some mechanisms for that ? >> How is that currrently handled ? > > The way fmc-bus is designed, does not need to discover FPGA devices. How then does the discovery ? Or is that purely application specific and entirely handled in userspace ? >> Could you put a dtb directly in the FPGA, at some defined address ? > > In general yes. Why not? Ok, than that's probably the way I would go. Not sure whether we already have standard mechanisms for that. > Well, I am suggesting to erase fmc-bus from the Linux kernel and to provide a > device_class for a standardized way to access that I2C EEPROM. I already have > the implementation; perhaps it needs some clean-up and for sure to be ported > to 5.x (I will do this job only if there is interest in removing/replacing the > current fmc subsystem). Can you post what you've already got ? I'd like to have a closer look at it. Maybe it has some overlap w/ pstore or firmware flashing. >> Do you happen to have some remote-accesible testbed for playground ? > > Of course not :D > CERN infrastructure is not freely accessible How sad :( I believe CERN could be one of the top candidates for setting up such environments - not for this case, but also other Linux-supported hardware. Do you happen to know some people there, with whom we could have a talk about that ? --mtx -- Enrico Weigelt, metux IT consult Free software and Linux embedded engineering info@metux.net -- +49-151-27565287
On 11.06.19 16:58, Alessandro Rubini wrote: Hi, > This FMC (fpga mezzanine carrier) is a "slot" for peripheral boards. > Like shields for beaglebone or such stuff. So there is a board > (and our own are actually pci-x and vme) with the FPGA, and the pins > are carried to this "mezzanine" where the I/O happens. The mezzanine > is usually just dumb, but it has the mandatory eeprom. Ah, the fpga directly terminates w/ pcix or vme ? Okay, now I'm begining to get a picture ... So, from *logical* point of view (leaving aside the physical aspects), we have either pcix or vme cards, carrying an eepromp and an fpga ? The i2c and gpio controllers are also implemented in the fpga ? > So, our "fine delay" board, that can timestamp pulses and generate > other pulses at precise times) can plug in both the pcix and vme carrier. Do the bitfiles differ between pcix and vme case ? > The tow major ones, IIUC, are (1) the same mezzanine would require > different bitstreams according to external information and (2) the > fpga binary itself my expose different "devices" where a generalized > driver would benefit, instead of a mezzanine-specific driver. Have you already tried the MFD approach, where a board (in this case: bitfile) specific driver instantiates several generic ones ? --mtx -- Enrico Weigelt, metux IT consult Free software and Linux embedded engineering info@metux.net -- +49-151-27565287
>> This FMC (fpga mezzanine carrier) is a "slot" for peripheral boards. >> Like shields for beaglebone or such stuff. So there is a board >> (and our own are actually pci-x and vme) with the FPGA, and the pins >> are carried to this "mezzanine" where the I/O happens. The mezzanine >> is usually just dumb, but it has the mandatory eeprom. > > Ah, the fpga directly terminates w/ pcix or vme ? The FMC standard doesn't talk about this. In our devices (pcix and vme adapters), there is a bridge. If the fpga was directly connected, it could not speak vme or pcix. Then there are standalone applications, but then it's not of interest for the linux kernel (there is no host). > So, from *logical* point of view (leaving aside the physical aspects), > we have either pcix or vme cards, carrying an eepromp and an fpga ? > The i2c and gpio controllers are also implemented in the fpga ? No. There is a "carrier" that has the fpga and possible the bridge to be a slave of a host. And a connector where the "mezzanine" fits. The mezzanine is like a shield of arduino/raspberry: it usually only hosts the signa conditioning and adc/dac. But it also has an eeprom. In the use case I worked with, we had an oscilloscope (4 adc, hifh speed) and a fine-delay card, both plugging in the same carrier (the pcix carrier). The pci driver for the carrier (which is a pci device) instantiates the fmc bus and loads a "generic" bitstream for the fpga. The bitstream is carrier-specific. With this I could read the eeprom, and thus know what the mezzanine was. This told me what "final" bitstream to load. After that, we have a "device" (adc, fine-delay, whatever) and thus could load the appropriate driver. The eeprom is read-only. Only exported in sysfs for informational purposes. This is the current fmc-bus design. But if there are no users (besides people building it off-tree), it can go. About SDB: We used SDB because ipmi-fru is a pain of complexity. Thus, we had a filesystem-like thing in the eeprom where we can store calibration parameters or whatever (we use SDB for mac addresses in flash, for example, with a barebox "driver" to read them). In fmc mezzanines we declare the "fru" information at offset 0 as a "file" within an sdb structure, whose magic number is at offset 256 or 512 in the eeprom. SDB was born to describe peripherals within a wishbone address space (so we autdetect peripherals from within our soft-core), and it lends itself ferfectly to describe an eeprom. It's more than a "partition table" thing and less than a filesystem. You can think about it as an array of "legacy uboot images", with subdirectories (wishbone crossbars) and relative addressing. Something very simple to parse from a microcontroller. Then we lacked the resources to properly advertize it, and it remained somehow an internal project. I don't know why federico says it's abandoned, becase it is still used to describe fpga address spaces in at least one german research center. Maybe it's just abandoned for fmc eeprom, I don't know. Please note that I'm not insisting to keep fmc-bus as is, i just try to explain the design choices, taken back when the world was easy. Now I'm basically out of these project, and whatever federico says is much more valuable than my out-of-date view of the issues. thanks /alessandro
diff --git a/drivers/fmc/fmc-core.c b/drivers/fmc/fmc-core.c index bbcb505d1522..f08b6f81f442 100644 --- a/drivers/fmc/fmc-core.c +++ b/drivers/fmc/fmc-core.c @@ -14,6 +14,7 @@ #include <linux/device.h> #include <linux/fmc.h> #include <linux/fmc-sdb.h> +#include <linux/gpio/consumer.h> #include "fmc-private.h" diff --git a/drivers/fmc/fmc-trivial.c b/drivers/fmc/fmc-trivial.c index 8defdee3e3a3..bab02d17f02c 100644 --- a/drivers/fmc/fmc-trivial.c +++ b/drivers/fmc/fmc-trivial.c @@ -15,7 +15,7 @@ #include <linux/module.h> #include <linux/init.h> #include <linux/interrupt.h> -#include <linux/gpio.h> +#include <linux/gpio/consumer.h> #include <linux/fmc.h> static struct fmc_driver t_drv; /* initialized later */ @@ -31,12 +31,12 @@ static irqreturn_t t_handler(int irq, void *dev_id) static struct fmc_gpio t_gpio[] = { { - .gpio = FMC_GPIO_IRQ(0), - .mode = GPIOF_DIR_IN, + .gpio_index = FMC_GPIO_IRQ(0), + .flags = GPIOD_IN, .irqmode = IRQF_TRIGGER_RISING, }, { - .gpio = FMC_GPIO_IRQ(1), - .mode = GPIOF_DIR_IN, + .gpio_index = FMC_GPIO_IRQ(1), + .flags = GPIOD_IN, .irqmode = IRQF_TRIGGER_RISING, } }; @@ -45,6 +45,7 @@ static int t_probe(struct fmc_device *fmc) { int ret; int index = 0; + int i; index = fmc_validate(fmc, &t_drv); if (index < 0) @@ -53,6 +54,16 @@ static int t_probe(struct fmc_device *fmc) ret = fmc_irq_request(fmc, t_handler, "fmc-trivial", IRQF_SHARED); if (ret < 0) return ret; + /* + * All GPIOs are associated with the FMC device using machine + * descriptor tables or similar. Pick the indices we want. + */ + for (i = 0; i < ARRAY_SIZE(t_gpio); i++) { + t_gpio[i].gpiod = devm_gpiod_get_index(&fmc->dev, + NULL, + t_gpio[i].gpio_index, + t_gpio[i].flags); + } /* ignore error code of call below, we really don't care */ fmc_gpio_config(fmc, t_gpio, ARRAY_SIZE(t_gpio)); diff --git a/include/linux/fmc.h b/include/linux/fmc.h index 3dc8a1b2db7b..452688aa97db 100644 --- a/include/linux/fmc.h +++ b/include/linux/fmc.h @@ -15,6 +15,7 @@ #include <linux/list.h> #include <linux/interrupt.h> #include <linux/io.h> +#include <linux/gpio/consumer.h> struct fmc_device; struct fmc_driver; @@ -99,9 +100,9 @@ struct fmc_driver { */ struct fmc_gpio { char *carrier_name; /* name or NULL for virtual pins */ - int gpio; - int _gpio; /* internal use by the carrier */ - int mode; /* GPIOF_DIR_OUT etc, from <linux/gpio.h> */ + int gpio_index; + struct gpio_desc *gpiod; + enum gpiod_flags flags; /* see <linux/gpio/consumer.h> */ int irqmode; /* IRQF_TRIGGER_LOW and so on */ }; @@ -115,14 +116,6 @@ struct fmc_gpio { #define FMC_GPIO_USER(x) ((x) + 0x1400) /* 256 of them */ /* We may add SCL and SDA, or other roles if the need arises */ -/* GPIOF_DIR_IN etc are missing before 3.0. copy from <linux/gpio.h> */ -#ifndef GPIOF_DIR_IN -# define GPIOF_DIR_OUT (0 << 0) -# define GPIOF_DIR_IN (1 << 0) -# define GPIOF_INIT_LOW (0 << 1) -# define GPIOF_INIT_HIGH (1 << 1) -#endif - /* * The operations are offered by each carrier and should make driver * design completely independent of the carrier. Named GPIO pins may be
This tries to convert the FMC subsystem to use GPIO descriptors. I say try because several pieces of this puzzle seems to not be in the mainline kernel. For details of this change, see drivers/gpio/TODO. We assume the FMC device is created somewhere and the GPIOs can be added in this place using gpio descriptor tables from <linux/gpio/machine.h> as in other conversions, but the place where the FMC device is created does not seem to be in the mainline Linux kernel. We assume the index ordering can be made to match the current GPIO index order in FMC which isn't especially specific. We get rid of some GPIO compatibility defines for kernel 3.0 in the process, it is long overdue. It might be that gpio numbers in this subsystem has nothing to do with gpio numbers in the GPIO subsystem, and it is just including <linux/gpio.h> and reusing some defines for no good reason. Then this should be changed to some approach decoupling FMC from the GPIO subsystem. Cc: Federico Vaga <federico.vaga@cern.ch> Cc: Pat Riehecky <riehecky@fnal.gov> Cc: Alessandro Rubini <rubini@gnudd.com> Signed-off-by: Linus Walleij <linus.walleij@linaro.org> --- drivers/fmc/fmc-core.c | 1 + drivers/fmc/fmc-trivial.c | 21 ++++++++++++++++----- include/linux/fmc.h | 15 ++++----------- 3 files changed, 21 insertions(+), 16 deletions(-) -- 2.20.1