Re: [PR] arch/risc-v: Add dedicated GPIO support for esp32[-s2|-s3|-c3|-c6|-h2] [nuttx]
eren-terzioglu commented on PR #16223: URL: https://github.com/apache/nuttx/pull/16223#issuecomment-2879251333 Thanks @raiden00pl, @acassis, @xiaoxiang781216. I updated without doing any change on driver level. -- 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]
Re: [PR] arch/risc-v: Add dedicated GPIO support for esp32[-s2|-s3|-c3|-c6|-h2] [nuttx]
xiaoxiang781216 commented on PR #16223: URL: https://github.com/apache/nuttx/pull/16223#issuecomment-2811852516 can we add ioctl to expose ioexpender_lowhalf to userspace? which has api to control pin in the group. -- 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]
Re: [PR] arch/risc-v: Add dedicated GPIO support for esp32[-s2|-s3|-c3|-c6|-h2] [nuttx]
acassis commented on PR #16223: URL: https://github.com/apache/nuttx/pull/16223#issuecomment-2809319506 > I'm not sure if expanding `/drivers/ioexpander/gpio.c` this way is a good idea. This driver is designed to support **single GPIO**. The interface for single GPIO is different and mutually exclusive with the interface to control many GPIOs. In your implementation, when `go_bundle_` is defined, all the rest of the operations in the gpio driver doesn't make sense. So what is the point of combining two not compatible implementations into one? > > The correct solution is a new driver, dedicated to control many GPIOs, maybe something like `gpiochip` in Linux. @eren-terzioglu I think @raiden00pl raised some important point here. Another thing that I noticed is the way it was implemented this solution seems ESP-specific. The right solution should be creating ARCH_HAS_GPIO_GROUPING and also the gpio example should have been modified to support it. I think GPIO grouping is more common term than bundle. If you decide to create a new driver, I suggest something like iogroup, gpiowide or par_io, it should be more appropriated, gpiochip that Raiden suggested is not self-explanatory. -- 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]
Re: [PR] arch/risc-v: Add dedicated GPIO support for esp32[-s2|-s3|-c3|-c6|-h2] [nuttx]
raiden00pl commented on PR #16223: URL: https://github.com/apache/nuttx/pull/16223#issuecomment-2809305448 > Should we implement a new driver which includes just 2 calls? Maybe we can write the driver when we implement parallel io interface for esp devices to make the interface more inclusive? Yes, these are two different functionalities and should be kept separate. What's the point of using these 2 calls from gpio driver if all others gpio methods are NULL and not compatible with "bundle" approach? I'm against shortcuts, it is against INVIOLABLES.md. We already have an example of such an easy way out in upstream and it doesn't seem like anyone wants to fix it: https://github.com/apache/nuttx/issues/15431 -- 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]
Re: [PR] arch/risc-v: Add dedicated GPIO support for esp32[-s2|-s3|-c3|-c6|-h2] [nuttx]
eren-terzioglu commented on PR #16223: URL: https://github.com/apache/nuttx/pull/16223#issuecomment-2809287865 > I'm not sure if expanding `/drivers/ioexpander/gpio.c` this way is a good idea. This driver is designed to support **single GPIO**. The interface for single GPIO is different and mutually exclusive with the interface to control many GPIOs. In your implementation, when `go_bundle_` is defined, all the rest of the operations in the gpio driver doesn't make sense. So what is the point of combining two not compatible implementations into one? > > The correct solution is a new driver, dedicated to control many GPIOs, maybe something like `gpiochip` in Linux. Should we implement a new driver which includes just 2 calls? Maybe we can write the driver when we implement parallel io interface for esp devices to make the interface more inclusive? -- 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]
