a-lunev commented on pull request #5012:
URL: https://github.com/apache/incubator-nuttx/pull/5012#issuecomment-996994491
> > > Yes that is the idea. It is a breaking change and has to be documented
very well. So that people can get back to working and know what slew rates they
had.
> >
> >
> > I'm thinking about two more possible approaches:
> > **Approach N2**: get rid of stm32h7x3xx_pinmap.h (and all similar files
for all families) at all.
>
> Did you mean get rid of the slew, drive type or the files themselves?
For approach N2 I meant getting rid of the files themselves.
>
> I would be opposed to getting rid of the files. There is too much info
that is valid and tested. I think the drive type is known by context of the pin
and is usually defined by the IP block. I.E. I2C is open drain - so there is no
reason to removes everything. Just board (layout) specified components of the
pin definitions I.E. Slew Rate, PU/PD etc.
(Not approach N2 then) Existing files like stm32h7x3xx_pinmap.h could be
announced deprecated and retained (say for 5 years) for compatibility purposes
with the boards that are out of NuttX tree. New pinmap files could be
introduced (that do not contain Slew Rate, PU/PD etc.) and recommended for use.
> > Let's consider one of the following potential change in
stm32h7x3xx_pinmap.h: before: `#define GPIO_SDMMC2_CMD
(GPIO_ALT|GPIO_AF9|GPIO_SPEED_50MHz|GPIO_PUSHPULL|GPIO_PORTA|GPIO_PIN0) `
after: `#define GPIO_SDMMC2_CMD_2 (GPIO_ALT|GPIO_AF9|GPIO_PORTA|GPIO_PIN0)`
> > And the following line is going to be added to board.h: `#define
GPIO_SDMMC2_CMD (GPIO_SDMMC2_CMD_2|GPIO_SPEED_50MHz|GPIO_PUSHPULL)`
> > Currently there are about 1,500 defines in stm32h7x3xx_pinmap.h and all
or part of 1,500 defines (depending on a particular board) are going to migrate
to board.h files.
> > Then if we look into board.h, "GPIO_SDMMC2_CMD_2" and all the other
referred constants may be confusing (because of "_ < number >" endings).
Possibly it would be better to have the whole (not split) define in board.h,
namely: `#define GPIO_SDMMC2_CMD
(GPIO_ALT|GPIO_AF9|GPIO_SPEED_50MHz|GPIO_PUSHPULL|GPIO_PORTA|GPIO_PIN0) ` In
this case it would be more clear how a pin is configured for given board.
> > **Approach N3**: the same as Approach N1 (the one you have suggested),
however use a port label instead of "_ < number > ". The potential change in
stm32h7x3xx_pinmap.h: before: `#define GPIO_SDMMC2_CMD
(GPIO_ALT|GPIO_AF9|GPIO_SPEED_50MHz|GPIO_PUSHPULL|GPIO_PORTA|GPIO_PIN0) `
after: `#define GPIO_SDMMC2_CMD_PA0 (GPIO_ALT|GPIO_AF9|GPIO_PORTA|GPIO_PIN0)`
> > And the following line is going to be added to board.h: `#define
GPIO_SDMMC2_CMD (GPIO_SDMMC2_CMD_PA0|GPIO_SPEED_50MHz|GPIO_PUSHPULL)`
> > What do you think?
>
> N3 - is just encoding the information in the define in the name. I am not
a fan.
>
> once all the defines have a _n all of this is a moot point.
Yes, there would be a huge rework. However, I think having port name+number
in the define name is much less error-prone compared to just _n.
> The NuttX drivers use the root name: GPIO_SDMMC2_CMD, I can role my own in
board.h.
>
> I can ignore the _n
>
> ```
> #define GPIO_SDMMC2_CMD
(GPIO_ALT|GPIO_AF9|GPIO_SPEED_50MHz|GPIO_PUSHPULL|GPIO_PORTA|GPIO_PIN0)
> ```
>
(as in Approach N2)
> I can use the _n and ADD
>
> ```
> #define GPIO_SDMMC2_CMD (GPIO_SDMMC2_CMD_2|GPIO_SPEED_50MHz|GPIO_PUSHPULL)
> ```
>
> Or change it, See
[PX4/PX4-Autopilot@de7aa78](https://github.com/PX4/PX4-Autopilot/commit/de7aa78518fe77a04789f4f8ab06fa420c74542e)
Yes, there are comments with port name+number clarification for each define.
That is one of the points why I think it'd better to use port name+number in
the define name instead of _n. It would be self-documented and then it would be
almost unlikely to misuse the constants.
>
> The simple fix is to remove all root names from the pin maps files by
adding the `_n` In the past @mubes and I had this discussion and I had
suggested using `_0` to - un-root the root names.
>
> It should only effect the root name defines in the pinmaps files, and that
will be much less then all of defines in the pin map.
Yes, I think it would be also a signal (compile errors) for out-of-tree
board maintainer to migrate to the new pinmap way.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]