You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@nuttx.apache.org by Grr <ge...@gmail.com> on 2021/01/08 04:19:53 UTC

SPI Driver Balkanization

I've been working towards creating a unified (character or SocketCAN)
driver model for SPI CAN controller MCP2515 and I found out that SPI_*
macro family allow platform-neutral SPI device control but there's one
thing I cannot find:

Is it possible to abstract chip select (CS) definition and usage in a
similar way?

Question arises from the fact that most SPI drivers are initialized (and
*de facto supported*) per board but many (if not most) boards offer SPI
external connectivity so any *collection* of SPI devices can be connected
to any such board

Since most drivers are initialized *per board*, any given *supported*
device cannot be used in any given board without initialization being
"ported" to that board, something a basic user cannot do

Another related problem (discussed between Gregory Nutt and Sebastien
Lorquet in
https://nuttx.yahoogroups.narkive.com/7qv88uHr/proposal-support-multiple-spi-devices-of-same-type)
is support of multiple similar devices in the same bus

The solution to problem A is to have an abstraction mechanism for CS that
can be used not only in driver code but in menuconfig so user is able to
configure any possible combination of boards and *external* busses and
devices to exploit NuttX device support.

Is there or could be such abstraction mechanism to complement SPI_*?

For problem B, I've seen hints in Bob Feretich's ADXL372 driver but
couldn't find use of it to learn how. I've seen device IDs have an index
but I still don't know how its used

Any hint or opinion is greatly appreciated

TIA

Grr

Re: SPI Driver Balkanization

Posted by Grr <ge...@gmail.com>.
Some drivers do do that.  They accept a "devno" argument in their init
> function and then pass it with the SPI calls.
>

If *every* driver *must* do that, problem solved


>
>
> On Fri, Jan 8, 2021 at 7:41 PM Brennan Ashton <ba...@brennanashton.com>
> wrote:
> >
> > On Fri, Jan 8, 2021, 10:35 AM Abdelatif Guettouche <
> > abdelatif.guettouche@gmail.com> wrote:
> >
> > > >  One problem was I didn't understand current CS definition. I thought
> > > > #define GPIO_MCP2515_CS
>  (GPIO_OUTPUT|GPIO_CNF_OUTPP|GPIO_MODE_50MHz|\
> > > >                            GPIO_OUTPUT_SET|GPIO_PORTA|GPIO_PIN4)
> > > > created a bitmask named GPIO_MCP2515_CS but no, it creates an ASCII
> > > string
> > > > that does its magic in stm32_configgpio()
> > >
> > >
> > > That doesn't create an ASCII string.  GPIOs follow a simple encoding
> > > that changes from chip to chip depending on the capabilities of the
> > > chip.
> > >
> > > The CS pin isn't part of the chip driver because it's (for the most
> > > cases) a normal GPIO that changes from board to board.
> > > The board logic that adds a CS pin is very simple and doesn't require
> > > deep kernel knowledge.
> > >
> >
> > A key exception is when the CS is behind an IO expander. Which the
> current
> > design allows.
> >
> > The biggest issue I see is that drivers are hard coding the CS reference
> > like the MCP2515 which always uses SPIDEV_CANBUS(0), this should be
> passed
> > in to the initialization of all SPI devices and owned by the private
> device
> > structure not hard coded. I could even see the argument for this being
> part
> > of the generic SPI interface structure.
> >
> > >
>

Re: SPI Driver Balkanization

Posted by Grr <ge...@gmail.com>.
That's exactly what I need: an example to follow. I'll check it

Thanks, Alan

Grr

El vie, 8 ene 2021 a las 13:10, Alan Carvalho de Assis (<ac...@gmail.com>)
escribió:

> On 1/8/21, Abdelatif Guettouche <ab...@gmail.com> wrote:
> >> The biggest issue I see is that drivers are hard coding the CS reference
> >> like the MCP2515 which always uses SPIDEV_CANBUS(0), this should be
> >> passed
> >> in to the initialization of all SPI devices and owned by the private
> >> device
> >> structure not hard coded. I could even see the argument for this being
> >> part
> >> of the generic SPI interface structure.
> >
> >
> > Some drivers do do that.  They accept a "devno" argument in their init
> > function and then pass it with the SPI calls.
> >
>
> Yes, an example is this one: nuttx/drivers/sensors/max31855.c
>
> When I created the mcp2515 driver I wasn't considering to use more
> than one CAN module by board. Sorry, my fault!
>
> BTW, the modification is simples. Grr, just look how it is done in the
> max31855 and do the same for mcp2515.
>
> The board initialization for max31855 is here:
> nuttx/boards/arm/stm32/common/src/stm32_max31855.c
>
> BR,
>
> Alan
>

Re: SPI Driver Balkanization

Posted by Alan Carvalho de Assis <ac...@gmail.com>.
On 1/8/21, Abdelatif Guettouche <ab...@gmail.com> wrote:
>> The biggest issue I see is that drivers are hard coding the CS reference
>> like the MCP2515 which always uses SPIDEV_CANBUS(0), this should be
>> passed
>> in to the initialization of all SPI devices and owned by the private
>> device
>> structure not hard coded. I could even see the argument for this being
>> part
>> of the generic SPI interface structure.
>
>
> Some drivers do do that.  They accept a "devno" argument in their init
> function and then pass it with the SPI calls.
>

Yes, an example is this one: nuttx/drivers/sensors/max31855.c

When I created the mcp2515 driver I wasn't considering to use more
than one CAN module by board. Sorry, my fault!

BTW, the modification is simples. Grr, just look how it is done in the
max31855 and do the same for mcp2515.

The board initialization for max31855 is here:
nuttx/boards/arm/stm32/common/src/stm32_max31855.c

BR,

Alan

Re: SPI Driver Balkanization

Posted by Abdelatif Guettouche <ab...@gmail.com>.
> The biggest issue I see is that drivers are hard coding the CS reference
> like the MCP2515 which always uses SPIDEV_CANBUS(0), this should be passed
> in to the initialization of all SPI devices and owned by the private device
> structure not hard coded. I could even see the argument for this being part
> of the generic SPI interface structure.


Some drivers do do that.  They accept a "devno" argument in their init
function and then pass it with the SPI calls.




On Fri, Jan 8, 2021 at 7:41 PM Brennan Ashton <ba...@brennanashton.com> wrote:
>
> On Fri, Jan 8, 2021, 10:35 AM Abdelatif Guettouche <
> abdelatif.guettouche@gmail.com> wrote:
>
> > >  One problem was I didn't understand current CS definition. I thought
> > > #define GPIO_MCP2515_CS   (GPIO_OUTPUT|GPIO_CNF_OUTPP|GPIO_MODE_50MHz|\
> > >                            GPIO_OUTPUT_SET|GPIO_PORTA|GPIO_PIN4)
> > > created a bitmask named GPIO_MCP2515_CS but no, it creates an ASCII
> > string
> > > that does its magic in stm32_configgpio()
> >
> >
> > That doesn't create an ASCII string.  GPIOs follow a simple encoding
> > that changes from chip to chip depending on the capabilities of the
> > chip.
> >
> > The CS pin isn't part of the chip driver because it's (for the most
> > cases) a normal GPIO that changes from board to board.
> > The board logic that adds a CS pin is very simple and doesn't require
> > deep kernel knowledge.
> >
>
> A key exception is when the CS is behind an IO expander. Which the current
> design allows.
>
> The biggest issue I see is that drivers are hard coding the CS reference
> like the MCP2515 which always uses SPIDEV_CANBUS(0), this should be passed
> in to the initialization of all SPI devices and owned by the private device
> structure not hard coded. I could even see the argument for this being part
> of the generic SPI interface structure.
>
> >

Re: SPI Driver Balkanization

Posted by Brennan Ashton <ba...@brennanashton.com>.
On Fri, Jan 8, 2021, 10:35 AM Abdelatif Guettouche <
abdelatif.guettouche@gmail.com> wrote:

> >  One problem was I didn't understand current CS definition. I thought
> > #define GPIO_MCP2515_CS   (GPIO_OUTPUT|GPIO_CNF_OUTPP|GPIO_MODE_50MHz|\
> >                            GPIO_OUTPUT_SET|GPIO_PORTA|GPIO_PIN4)
> > created a bitmask named GPIO_MCP2515_CS but no, it creates an ASCII
> string
> > that does its magic in stm32_configgpio()
>
>
> That doesn't create an ASCII string.  GPIOs follow a simple encoding
> that changes from chip to chip depending on the capabilities of the
> chip.
>
> The CS pin isn't part of the chip driver because it's (for the most
> cases) a normal GPIO that changes from board to board.
> The board logic that adds a CS pin is very simple and doesn't require
> deep kernel knowledge.
>

A key exception is when the CS is behind an IO expander. Which the current
design allows.

The biggest issue I see is that drivers are hard coding the CS reference
like the MCP2515 which always uses SPIDEV_CANBUS(0), this should be passed
in to the initialization of all SPI devices and owned by the private device
structure not hard coded. I could even see the argument for this being part
of the generic SPI interface structure.

>

Re: SPI Driver Balkanization

Posted by Abdelatif Guettouche <ab...@gmail.com>.
>  One problem was I didn't understand current CS definition. I thought
> #define GPIO_MCP2515_CS   (GPIO_OUTPUT|GPIO_CNF_OUTPP|GPIO_MODE_50MHz|\
>                            GPIO_OUTPUT_SET|GPIO_PORTA|GPIO_PIN4)
> created a bitmask named GPIO_MCP2515_CS but no, it creates an ASCII string
> that does its magic in stm32_configgpio()


That doesn't create an ASCII string.  GPIOs follow a simple encoding
that changes from chip to chip depending on the capabilities of the
chip.

The CS pin isn't part of the chip driver because it's (for the most
cases) a normal GPIO that changes from board to board.
The board logic that adds a CS pin is very simple and doesn't require
deep kernel knowledge.




On Fri, Jan 8, 2021 at 4:45 PM Grr <ge...@gmail.com> wrote:
>
> Thank you for the response
>
> Hi,
> > some of the points you mention about drivers being tied to board logic and
> > the fact that initialization is "hard coded" is something I once tried to
> > improve
> > when I worked on "device tree" support (discussion at
> > https://github.com/apache/incubator-nuttx/issues/1020). However as this
> > was
> > quite a big undertaking it was shelved for the time being. In that regard,
> > what I can say is:
> >
> > - With the introduction of "common board logic", drivers support can be
> > implemented
> > at the board-family level (e.g.: stm32). This reduces the necessary code
> > to support
> > the driver in another board of the family to a single initialization call
> > and pin definitions
> >
> > - Right now NuttX usually ties resource definition ("does the bard have
> > device X") to driver
> > support ("is driver for device X enabled") by conditional compilation on
> > board logic initialization.
> > So, multiple instances of a device require custom handling (besides the
> > fact that the driver needs
> > to indeed support multiple instances).
> >
> > As a result of the above, indeed trying to expose a device usually
> > requires changing code
> > in board logic. With devicetree I tried to tackle that but I realized
> > previous steps were
> > required to improve this in NuttX (first one was to reduce duplicated
> > code). For this reason
> > I still have on my bucket list to improve general handling of device
> > drivers.
>
>
> The way I envisioned
> > is to go towards an unified driver model/interface based on callbacks
> > (register, initialize, uninitialize).
> > If all drivers implement this, initialization can be handled by generic
> > code invoking the callbacks
> > (from all instances of drivers defined in a constant array) at a higher
> > level than board logic.
> >
>
> You mean what spi_ops_s already does but in a general way. That sounds
> great and I was considering something like that to generalize Peter van der
> Perk's SocketCAN driver and use it with different CAN controllers
>
> struct spi_ops_s
> {
>   CODE int      (*lock)(FAR struct spi_dev_s *dev, bool lock);
>   CODE void     (*select)(FAR struct spi_dev_s *dev, uint32_t devid,
>                   bool selected);
>   CODE uint32_t (*setfrequency)(FAR struct spi_dev_s *dev,
>                   uint32_t frequency);
>   CODE int      (*setdelay)(FAR struct spi_dev_s *dev, uint32_t a,
>                   uint32_t b, uint32_t c);
> ...
> }
>
> The final step to handle all previous mentioned limitations would be to use
> > device tree to declare
> > existing resources and generate the driver-instance array automatically.
> > As device trees naturally
> > handle overlaying, this would allow for a base resource definition for all
> > on-board devices and then
> > have the user to extend/override this with some user-defined device tree.
> > In there you could add off-board
> > devices, which also enables support for multiple instances.
> >
>
> That's exactly  what I want, albeit not as ambitious. My guess is it can be
> half accomplished right now with CS abstraction, Kconfig magic and defining
> a standard model for driver structure to which existing drivers can be
> slowly migrated
>
> One problem was I didn't understand current CS definition. I thought
>
> #define GPIO_MCP2515_CS   (GPIO_OUTPUT|GPIO_CNF_OUTPP|GPIO_MODE_50MHz|\
>                            GPIO_OUTPUT_SET|GPIO_PORTA|GPIO_PIN4)
>
> created a bitmask named GPIO_MCP2515_CS but no, it creates an ASCII string
> that does its magic in stm32_configgpio()
>
> My question is if the whole *_configgpio() family does the same everywhere
> so GPIO_MCP2515_CS can be applied correctly to any _configgpio() function
> and it's only a matter of generalizing the existing structure. In that case
> Brennan is correct it's already abstracted and it's only a matter of using
> it to avoid board "lock-in".
>
> >
> > Hopefully sometime in the future we can discuss something along these
> > lines to be added to NuttX as
> > I think it would be very valuable.
> >
>
> Maybe we could start now by creating the seed of the driver model and the
> user config part. That would be very nice and, I hope, very achievable
>
> TIA
> Grr
>
>
> Best,
> > Matias
> >
>
>
>
> >
> > On Fri, Jan 8, 2021, at 01:19, Grr wrote:
> > > I've been working towards creating a unified (character or SocketCAN)
> > > driver model for SPI CAN controller MCP2515 and I found out that SPI_*
> > > macro family allow platform-neutral SPI device control but there's one
> > > thing I cannot find:
> > >
> > > Is it possible to abstract chip select (CS) definition and usage in a
> > > similar way?
> > >
> > > Question arises from the fact that most SPI drivers are initialized (and
> > > *de facto supported*) per board but many (if not most) boards offer SPI
> > > external connectivity so any *collection* of SPI devices can be connected
> > > to any such board
> > >
> > > Since most drivers are initialized *per board*, any given *supported*
> > > device cannot be used in any given board without initialization being
> > > "ported" to that board, something a basic user cannot do
> > >
> > > Another related problem (discussed between Gregory Nutt and Sebastien
> > > Lorquet in
> > >
> > https://nuttx.yahoogroups.narkive.com/7qv88uHr/proposal-support-multiple-spi-devices-of-same-type
> > )
> > > is support of multiple similar devices in the same bus
> > >
> > > The solution to problem A is to have an abstraction mechanism for CS that
> > > can be used not only in driver code but in menuconfig so user is able to
> > > configure any possible combination of boards and *external* busses and
> > > devices to exploit NuttX device support.
> > >
> > > Is there or could be such abstraction mechanism to complement SPI_*?
> > >
> > > For problem B, I've seen hints in Bob Feretich's ADXL372 driver but
> > > couldn't find use of it to learn how. I've seen device IDs have an index
> > > but I still don't know how its used
> > >
> > > Any hint or opinion is greatly appreciated
> > >
> > > TIA
> > >
> > > Grr
> > >
> >

Re: SPI Driver Balkanization

Posted by Grr <ge...@gmail.com>.
Thank you for the response

Hi,
> some of the points you mention about drivers being tied to board logic and
> the fact that initialization is "hard coded" is something I once tried to
> improve
> when I worked on "device tree" support (discussion at
> https://github.com/apache/incubator-nuttx/issues/1020). However as this
> was
> quite a big undertaking it was shelved for the time being. In that regard,
> what I can say is:
>
> - With the introduction of "common board logic", drivers support can be
> implemented
> at the board-family level (e.g.: stm32). This reduces the necessary code
> to support
> the driver in another board of the family to a single initialization call
> and pin definitions
>
> - Right now NuttX usually ties resource definition ("does the bard have
> device X") to driver
> support ("is driver for device X enabled") by conditional compilation on
> board logic initialization.
> So, multiple instances of a device require custom handling (besides the
> fact that the driver needs
> to indeed support multiple instances).
>
> As a result of the above, indeed trying to expose a device usually
> requires changing code
> in board logic. With devicetree I tried to tackle that but I realized
> previous steps were
> required to improve this in NuttX (first one was to reduce duplicated
> code). For this reason
> I still have on my bucket list to improve general handling of device
> drivers.


The way I envisioned
> is to go towards an unified driver model/interface based on callbacks
> (register, initialize, uninitialize).
> If all drivers implement this, initialization can be handled by generic
> code invoking the callbacks
> (from all instances of drivers defined in a constant array) at a higher
> level than board logic.
>

You mean what spi_ops_s already does but in a general way. That sounds
great and I was considering something like that to generalize Peter van der
Perk's SocketCAN driver and use it with different CAN controllers

struct spi_ops_s
{
  CODE int      (*lock)(FAR struct spi_dev_s *dev, bool lock);
  CODE void     (*select)(FAR struct spi_dev_s *dev, uint32_t devid,
                  bool selected);
  CODE uint32_t (*setfrequency)(FAR struct spi_dev_s *dev,
                  uint32_t frequency);
  CODE int      (*setdelay)(FAR struct spi_dev_s *dev, uint32_t a,
                  uint32_t b, uint32_t c);
...
}

The final step to handle all previous mentioned limitations would be to use
> device tree to declare
> existing resources and generate the driver-instance array automatically.
> As device trees naturally
> handle overlaying, this would allow for a base resource definition for all
> on-board devices and then
> have the user to extend/override this with some user-defined device tree.
> In there you could add off-board
> devices, which also enables support for multiple instances.
>

That's exactly  what I want, albeit not as ambitious. My guess is it can be
half accomplished right now with CS abstraction, Kconfig magic and defining
a standard model for driver structure to which existing drivers can be
slowly migrated

One problem was I didn't understand current CS definition. I thought

#define GPIO_MCP2515_CS   (GPIO_OUTPUT|GPIO_CNF_OUTPP|GPIO_MODE_50MHz|\
                           GPIO_OUTPUT_SET|GPIO_PORTA|GPIO_PIN4)

created a bitmask named GPIO_MCP2515_CS but no, it creates an ASCII string
that does its magic in stm32_configgpio()

My question is if the whole *_configgpio() family does the same everywhere
so GPIO_MCP2515_CS can be applied correctly to any _configgpio() function
and it's only a matter of generalizing the existing structure. In that case
Brennan is correct it's already abstracted and it's only a matter of using
it to avoid board "lock-in".

>
> Hopefully sometime in the future we can discuss something along these
> lines to be added to NuttX as
> I think it would be very valuable.
>

Maybe we could start now by creating the seed of the driver model and the
user config part. That would be very nice and, I hope, very achievable

TIA
Grr


Best,
> Matias
>



>
> On Fri, Jan 8, 2021, at 01:19, Grr wrote:
> > I've been working towards creating a unified (character or SocketCAN)
> > driver model for SPI CAN controller MCP2515 and I found out that SPI_*
> > macro family allow platform-neutral SPI device control but there's one
> > thing I cannot find:
> >
> > Is it possible to abstract chip select (CS) definition and usage in a
> > similar way?
> >
> > Question arises from the fact that most SPI drivers are initialized (and
> > *de facto supported*) per board but many (if not most) boards offer SPI
> > external connectivity so any *collection* of SPI devices can be connected
> > to any such board
> >
> > Since most drivers are initialized *per board*, any given *supported*
> > device cannot be used in any given board without initialization being
> > "ported" to that board, something a basic user cannot do
> >
> > Another related problem (discussed between Gregory Nutt and Sebastien
> > Lorquet in
> >
> https://nuttx.yahoogroups.narkive.com/7qv88uHr/proposal-support-multiple-spi-devices-of-same-type
> )
> > is support of multiple similar devices in the same bus
> >
> > The solution to problem A is to have an abstraction mechanism for CS that
> > can be used not only in driver code but in menuconfig so user is able to
> > configure any possible combination of boards and *external* busses and
> > devices to exploit NuttX device support.
> >
> > Is there or could be such abstraction mechanism to complement SPI_*?
> >
> > For problem B, I've seen hints in Bob Feretich's ADXL372 driver but
> > couldn't find use of it to learn how. I've seen device IDs have an index
> > but I still don't know how its used
> >
> > Any hint or opinion is greatly appreciated
> >
> > TIA
> >
> > Grr
> >
>

Re: SPI Driver Balkanization

Posted by "Matias N." <ma...@imap.cc>.
Hi,
some of the points you mention about drivers being tied to board logic and 
the fact that initialization is "hard coded" is something I once tried to improve
when I worked on "device tree" support (discussion at
https://github.com/apache/incubator-nuttx/issues/1020). However as this was
quite a big undertaking it was shelved for the time being. In that regard,
what I can say is:

- With the introduction of "common board logic", drivers support can be implemented
at the board-family level (e.g.: stm32). This reduces the necessary code to support
the driver in another board of the family to a single initialization call and pin definitions

- Right now NuttX usually ties resource definition ("does the bard have device X") to driver
support ("is driver for device X enabled") by conditional compilation on board logic initialization.
So, multiple instances of a device require custom handling (besides the fact that the driver needs
to indeed support multiple instances).

As a result of the above, indeed trying to expose a device usually requires changing code
in board logic. With devicetree I tried to tackle that but I realized previous steps were
required to improve this in NuttX (first one was to reduce duplicated code). For this reason
I still have on my bucket list to improve general handling of device drivers. The way I envisioned
is to go towards an unified driver model/interface based on callbacks (register, initialize, uninitialize).
If all drivers implement this, initialization can be handled by generic code invoking the callbacks
(from all instances of drivers defined in a constant array) at a higher level than board logic.

The final step to handle all previous mentioned limitations would be to use device tree to declare
existing resources and generate the driver-instance array automatically. As device trees naturally
handle overlaying, this would allow for a base resource definition for all on-board devices and then
have the user to extend/override this with some user-defined device tree. In there you could add off-board
devices, which also enables support for multiple instances.

Hopefully sometime in the future we can discuss something along these lines to be added to NuttX as
I think it would be very valuable.

Best,
Matias

On Fri, Jan 8, 2021, at 01:19, Grr wrote:
> I've been working towards creating a unified (character or SocketCAN)
> driver model for SPI CAN controller MCP2515 and I found out that SPI_*
> macro family allow platform-neutral SPI device control but there's one
> thing I cannot find:
> 
> Is it possible to abstract chip select (CS) definition and usage in a
> similar way?
> 
> Question arises from the fact that most SPI drivers are initialized (and
> *de facto supported*) per board but many (if not most) boards offer SPI
> external connectivity so any *collection* of SPI devices can be connected
> to any such board
> 
> Since most drivers are initialized *per board*, any given *supported*
> device cannot be used in any given board without initialization being
> "ported" to that board, something a basic user cannot do
> 
> Another related problem (discussed between Gregory Nutt and Sebastien
> Lorquet in
> https://nuttx.yahoogroups.narkive.com/7qv88uHr/proposal-support-multiple-spi-devices-of-same-type)
> is support of multiple similar devices in the same bus
> 
> The solution to problem A is to have an abstraction mechanism for CS that
> can be used not only in driver code but in menuconfig so user is able to
> configure any possible combination of boards and *external* busses and
> devices to exploit NuttX device support.
> 
> Is there or could be such abstraction mechanism to complement SPI_*?
> 
> For problem B, I've seen hints in Bob Feretich's ADXL372 driver but
> couldn't find use of it to learn how. I've seen device IDs have an index
> but I still don't know how its used
> 
> Any hint or opinion is greatly appreciated
> 
> TIA
> 
> Grr
> 

Re: SPI Driver Balkanization

Posted by Grr <ge...@gmail.com>.
This is board and arch specific and cannot be more generalized than that.
> They will have different requirements for pullups, speed, etc.  The define
> should also start with BOARD_ but some use GPIO_.  This is the case
> for all PINs including UART, GPIO, SPI, USB, etc...
>

I mean after that arch and board-specific config, you end up with a list of
general pins available (CS1, CS2, CS3...) which are equivalent to SPI_* in
that they're valid everywhere and can be used despite architecture, SPI
master or bus number


> and the drivers will already have conditionals on those being defined.
>

Just like SPI_* don't break

>
> SPI is just a little special because you dont _need_ a CS gpio pin,
> but frequently you want it.


You need CS pins when you have more than one SPI device in a bus (something
very frequent). For example, all this comes from the fact that I'm
designing a control system that would benefit from having two CAN busses
instead of just one


> So you end up with the board specific hooks
> for what pin to turn on.
> as is here
>
> https://github.com/apache/incubator-nuttx/blob/cdd111a1faec9b40b707797e00c4afae4956fb3f/boards/arm/stm32/nucleo-f4x1re/src/stm32_spi.c#L140
>
> SPI drivers cannot depend on a fixed name for the CS pin because you may
> have multiple instances (what I thought you were asking to support)
>
> It's a fixed name in the sense CS1 is defined in all boards and archs so I
can write my driver using SPI_SELECT with CS1 and it will work everywhere


> You just update the define in your board.h file to include a mapping for
> what
> is wired up to pin 5.


I cannot do that if I'm not a kernel developer

>

> When you are bringing up your board one of the first
> things you should do is define the pins in your board.h
>

I'm not bringing up a new board but connecting an *external* supported chip
to a supported board via a standard bus


> Changing how pins are defined would break everyone so that is unlikely to
> change, and it is also not really something that can easily be mapped to
> Kconfig.
>

It doesn't break anything if it's parallel to everything just like SPI_*
macros are

Grr

Re: SPI Driver Balkanization

Posted by Brennan Ashton <ba...@brennanashton.com>.
On Thu, Jan 7, 2021 at 9:51 PM Grr <ge...@gmail.com> wrote:
>
> It is abstracted, for example see this file.
>
> >
> > https://github.com/apache/incubator-nuttx/blob/cdd111a1faec9b40b707797e00c4afae4956fb3f/boards/arm/stm32/nucleo-f4x1re/src/stm32_spi.c#L140
> >
>
> You mean  GPIO_MCP2515_CS? I don't quite understand its definition
>
> #define GPIO_MCP2515_CS   (GPIO_OUTPUT|GPIO_OTYPER_PP(0)|GPIO_SPEED_2MHz|\
>                            GPIO_OUTPUT_SET|GPIO_PORTA|GPIO_PIN4)
>
> I guess it's a way to combine different configuration bits with pin 4.
>
> Problem is it's only defined for pin 4 and only in a handful of STM32
> boards.

This is board and arch specific and cannot be more generalized than that.
They will have different requirements for pullups, speed, etc.  The define
should also start with BOARD_ but some use GPIO_.  This is the case
for all PINs including UART, GPIO, SPI, USB, etc...

Most systems besides SPI or GPIO you wont have to directly pass this
pin config you just define something like

#define BOARD_UART_0_TX_PIN (GPIO_INPUT | GPIO_PULLUP | GPIO_FUNC_UART
| GPIO_PIN16)
#define BOARD_UART_1_RX_PIN (GPIO_INPUT | GPIO_PULLUP | GPIO_FUNC_UART
| GPIO_PIN3)

and the drivers will already have conditionals on those being defined.

SPI is just a little special because you dont _need_ a CS gpio pin,
but frequently you want it.  So you end up with the board specific hooks
for what pin to turn on.
as is here
https://github.com/apache/incubator-nuttx/blob/cdd111a1faec9b40b707797e00c4afae4956fb3f/boards/arm/stm32/nucleo-f4x1re/src/stm32_spi.c#L140

SPI drivers cannot depend on a fixed name for the CS pin because you may
have multiple instances (what I thought you were asking to support)

>
> What happens if I want to use that driver with a PIC32MX board and I'm not
> a kernel developer?
> What happens if pin 4 is already used and I *need* to use pin 5 and again
> I'm not a kernel developer?

You just update the define in your board.h file to include a mapping for what
is wired up to pin 5.  When you are bringing up your board one of the first
things you should do is define the pins in your board.h

>
> My idea is to bring that level of configuration ability into menuconfig
>
> Are those GPIO_* macros work across architectures so they can be brought to
> menuconfig and combined there in a newbie-friendly way? It seems
> GPIO_OUTPUT exists in all architectures but I don't know it if works the
> same way everywhere

Changing how pins are defined would break everyone so that is unlikely to
change, and it is also not really something that can easily be mapped to
Kconfig.

You already have to add the initialization logic and this is just
another step in that.

--Brennan

Re: SPI Driver Balkanization

Posted by Grr <ge...@gmail.com>.
It is abstracted, for example see this file.

>
> https://github.com/apache/incubator-nuttx/blob/cdd111a1faec9b40b707797e00c4afae4956fb3f/boards/arm/stm32/nucleo-f4x1re/src/stm32_spi.c#L140
>

You mean  GPIO_MCP2515_CS? I don't quite understand its definition

#define GPIO_MCP2515_CS   (GPIO_OUTPUT|GPIO_OTYPER_PP(0)|GPIO_SPEED_2MHz|\
                           GPIO_OUTPUT_SET|GPIO_PORTA|GPIO_PIN4)

I guess it's a way to combine different configuration bits with pin 4.

Problem is it's only defined for pin 4 and only in a handful of STM32
boards.

What happens if I want to use that driver with a PIC32MX board and I'm not
a kernel developer?
What happens if pin 4 is already used and I *need* to use pin 5 and again
I'm not a kernel developer?

My idea is to bring that level of configuration ability into menuconfig

Are those GPIO_* macros work across architectures so they can be brought to
menuconfig and combined there in a newbie-friendly way? It seems
GPIO_OUTPUT exists in all architectures but I don't know it if works the
same way everywhere

Re: SPI Driver Balkanization

Posted by Brennan Ashton <ba...@brennanashton.com>.
On Thu, Jan 7, 2021 at 8:20 PM Grr <ge...@gmail.com> wrote:
>
> I've been working towards creating a unified (character or SocketCAN)
> driver model for SPI CAN controller MCP2515 and I found out that SPI_*
> macro family allow platform-neutral SPI device control but there's one
> thing I cannot find:
>
> Is it possible to abstract chip select (CS) definition and usage in a
> similar way?

It is abstracted, for example see this file.
https://github.com/apache/incubator-nuttx/blob/cdd111a1faec9b40b707797e00c4afae4956fb3f/boards/arm/stm32/nucleo-f4x1re/src/stm32_spi.c#L140

There is a case for each device on the SPI bus and with that the driver.

>
> Question arises from the fact that most SPI drivers are initialized (and
> *de facto supported*) per board but many (if not most) boards offer SPI
> external connectivity so any *collection* of SPI devices can be connected
> to any such board
>
> Since most drivers are initialized *per board*, any given *supported*
> device cannot be used in any given board without initialization being
> "ported" to that board, something a basic user cannot do

The MCP2515 dynamically allocates so this is not a problem:
https://github.com/apache/incubator-nuttx/blob/c8ff295d59c0360943abcaf19177cd0556f37f47/drivers/can/mcp2515.c#L2529

The problem here is that it hard codes the chip select:
https://github.com/apache/incubator-nuttx/blob/c8ff295d59c0360943abcaf19177cd0556f37f47/drivers/can/mcp2515.c#L333

This number (0) should be held by the driver structure and passed in
at instantiation time.  This would be a welcome change.
Then you can in your boards spi logic I mentioned above have
SPIDEV_CANBUS(0)
and
SPIDEV_CANBUS(1)
and this will all just settle out.


Does this address your concern?

--Brennan