Hi All. Sorry for the lack of documentation and clarity in my original submission. Thank you all for the feedback, taking it all on board and will come back with a more complete and acceptable submission.
I realised the original ticket number that I quoted was incorrect, #4869 is the correct number. Thanks for your understanding and patience, Aaron. ------- Original Message ------- On Saturday, March 18th, 2023 at 8:26 AM, Will <nyphb...@gmail.com> wrote: > On Fri, Mar 17, 2023 at 4:24 PM Gedare Bloom <ged...@rtems.org> wrote: > > > Thank you Aaron for contributing this. I have a few comments below, > > and I'll also give some comments on your code shortly. > > > > On Fri, Mar 17, 2023 at 1:34 AM Sebastian Huber > > <sebastian.hu...@embedded-brains.de> wrote: > > > > > > > > > > > > On 16.03.23 23:07, Chris Johns wrote: > > > > On 16/3/2023 6:13 pm, Sebastian Huber wrote: > > > >> Hello Aaron, > > > >> > > > >> this API seems to be RTEMS-specific. Maybe we should simply pick up an > > > >> existing > > > >> solution which is in more wide spread use, for example: > > > >> > > > >> https://docs.zephyrproject.org/latest/hardware/peripherals/flash.html > > > > > > > > That interface seems Zepher specific and looks to me like a series of > > > > calls that > > > > appear reasonable as a list to cover. > > > > > > The Zephyr API has drivers for a couple of chips: > > > > > > https://github.com/zephyrproject-rtos/zephyr/tree/main/drivers/flash > > > > > That code looks very zephyr-oriented. I think it would be a challenge > > to go first toward porting this API, if it requires substantial > > modifications of the upstream drivers to also use them, then it is > > more of a maintenance burden than the probable benefits of the code > > reuse. As a starting point, I do see a bespoke RTEMS implementation as > > a suitable place to begin. But, it would be good to understand the > > alternative flash driver frameworks that are out there, and what may > > be pros/cons of adopting them. > > > > > > The patch Aaron has posted is a driver. I > > > > prefer a driver like we have for I2C, SPI etc because the of support it > > > > brings. > > > > > > The I2C and SPI APIs are ported from Linux, so not RTEMS-specific. > > > > > > > > > > > The initial set of ioctl commands is small to start with. If you feel > > > > we should > > > > offer more I suggest they get added once this is merged. > > > > > > > > Is the change OK? > > > > > > I am not opposed to commit this API, however, I don't think it is a step > > > forward. It is yet another RTEMS-specific API. It has no related > > > documentation patches. The API has no high level user (for example > > > JAFFS2). The API has no implementation. It has no tests. It has no > > > driver framework (ONFI, JEDEC). It has no example driver. It has no > > > Doxygen documentation. The coding style of the new file has nothing to > > > do with the RTEMS coding style. > > > > > > > I think the provided code appears to be a good starting point. I > > would request that you might add Doxygen something aligned with what > > we have for the i2c or spi would be good. > > https://docs.rtems.org/doxygen/branches/master/group__I2C.html > > > > In addition to the shell command addition, can you create/extend a > > testsuite for exercising the API? > > > > I will address the code directly also, but these high-level additions > > will greatly improve the submission of a new API. Unfortunately, my > > work on code formatting is not quite complete enough to rely on it for > > fixing any style issues. I'll do what I can to guide you through that. > > > Aaron, > Just in case you haven't already found it, the document you should be using > for style and header documentation is here: > https://docs.rtems.org/branches/master/eng/coding-conventions.html > > Kinsey _______________________________________________ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
