Hi Peter, that is the same flow I was thinking of. I will create a PR affecting all the existing drivers.
Thanks, Carlos On Fri, Apr 11, 2025 at 1:42 PM Peter van der Perk <peter.vanderp...@nxp.com> wrote: > > Hi Carlos, > > I think your concern is valid and that the current implementation of > SIOCSCANBITRATE is a bugged. > > I think the solution would be to only allow SIOCSCANBITRATE when the > interface is down. > > My proposal would be that SIOCSCANBITRATE checks if the interface is already > up and if that's the case it should return -EBUSY. > Then the automatic ifup action that after SIOCSCANBITRATE that currently > happens should be removed. > > The correct procedure to change bitrate would be then > 1. ifdown the interface is it's up > 2. Call SIOCSCANBITRATE > 3. ifup the interface. > > Yours sincerely, > > Peter van der Perk > > -----Original Message----- > From: Carlos Sanchez <carlossanc...@geotab.com.INVALID> > Sent: Friday, April 11, 2025 12:16 PM > To: dev@nuttx.apache.org > Subject: socketcan ioctl(...SIOCSCANBITRATE...) brings the interface up > > > Hi devs, > > Short history: in all the existing socketcan drivers in Nuttx (imxrt, > s32k3xx, s32k1xx, stm32h7 and kinetis), the driver-specific implementation of > ioctl(...SIOCSCANBITRATE...) calls ifup at the end. > Is there a rationale / spec for this? > > Long history: > > In Linux, CAN bitrate is set with netlink, not ioctl, and you need to set a > bitrate before bringing the interface up (make sense). In Nuttx, you can > begin the interface up (it uses a default bitrate) and then you can change > it. My guess is calling ifup at the end is precisely to avoid requiring > another ifdown/ifup cycle from app code. > > SocketCAN is a Linux thing, so we should try to mimic the behaviour. > Implementing netlink would be overkill (and moreover, it is common in Linux > to use libsocketcan to abstract the netlink internals anyways) but at least > we should have similar semantics. In our use case, we have a portable > Linux/Nuttx app which uses socketcan, we have a thin compatibility layer for > Linux/Nuttx differences (so bitrate setting uses icotl on Nuttx and > libsocketcan in Linux) but it is difficult to make such a thin layer > accommodate this difference (it would need to store the setting, and only > call the ioctl just before bringing the interface up... essentially > duplicating the driver functionality). > > Couldn't we (Nuttx): > 1) Make SIOCSCANBITRATE just set the baudrate (and avoid changing interface > state)? It could even fail if the interface is already up (AFAIK this is what > netlink would do). > 2) Provide a slightly more detailed documentation of Nuttx-specific SocketCAN > ioctls (like, specifying whether the interface state is changed under the > hood) > > I volunteer to make the change, but this might affect application space so > I'd like to know what others' thoughts are. > > BR > > Carlos -- Carlos Sanchez (he, him, his) Geotab Senior Team Lead, Embedded Development | Europe Visit www.geotab.com Twitter | Facebook | YouTube | LinkedIn
