<div dir="ltr"><div dir="ltr"></div>On Fri, Mar 17, 2023 at 4:24 PM Gedare Bloom <<a href="mailto:gedare@rtems.org">gedare@rtems.org</a>> wrote:<div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Thank you Aaron for contributing this. I have a few comments below,<br>
and I'll also give some comments on your code shortly.<br>
<br>
On Fri, Mar 17, 2023 at 1:34 AM Sebastian Huber<br>
<<a href="mailto:sebastian.huber@embedded-brains.de" target="_blank">sebastian.huber@embedded-brains.de</a>> wrote:<br>
><br>
><br>
><br>
> On 16.03.23 23:07, Chris Johns wrote:<br>
> > On 16/3/2023 6:13 pm, Sebastian Huber wrote:<br>
> >> Hello Aaron,<br>
> >><br>
> >> this API seems to be RTEMS-specific. Maybe we should simply pick up an existing<br>
> >> solution which is in more wide spread use, for example:<br>
> >><br>
> >> <a href="https://docs.zephyrproject.org/latest/hardware/peripherals/flash.html" rel="noreferrer" target="_blank">https://docs.zephyrproject.org/latest/hardware/peripherals/flash.html</a><br>
> ><br>
> > That interface seems Zepher specific and looks to me like a series of calls that<br>
> > appear reasonable as a list to cover.<br>
><br>
> The Zephyr API has drivers for a couple of chips:<br>
><br>
> <a href="https://github.com/zephyrproject-rtos/zephyr/tree/main/drivers/flash" rel="noreferrer" target="_blank">https://github.com/zephyrproject-rtos/zephyr/tree/main/drivers/flash</a><br>
><br>
That code looks very zephyr-oriented. I think it would be a challenge<br>
to go first toward porting this API, if it requires substantial<br>
modifications of the upstream drivers to also use them, then it is<br>
more of a maintenance burden than the probable benefits of the code<br>
reuse. As a starting point, I do see a bespoke RTEMS implementation as<br>
a suitable place to begin. But, it would be good to understand the<br>
alternative flash driver frameworks that are out there, and what may<br>
be pros/cons of adopting them.<br>
<br>
> > The patch Aaron has posted is a driver. I<br>
> > prefer a driver like we have for I2C, SPI etc because the of support it brings.<br>
><br>
> The I2C and SPI APIs are ported from Linux, so not RTEMS-specific.<br>
><br>
> ><br>
> > The initial set of ioctl commands is small to start with. If you feel we should<br>
> > offer more I suggest they get added once this is merged.<br>
> ><br>
> > Is the change OK?<br>
><br>
> I am not opposed to commit this API, however, I don't think it is a step<br>
> forward. It is yet another RTEMS-specific API. It has no related<br>
> documentation patches. The API has no high level user (for example<br>
> JAFFS2). The API has no implementation. It has no tests. It has no<br>
> driver framework (ONFI, JEDEC). It has no example driver. It has no<br>
> Doxygen documentation. The coding style of the new file has nothing to<br>
> do with the RTEMS coding style.<br>
><br>
<br>
I think the provided code appears to be a good starting point.  I<br>
would request that you might add Doxygen something aligned with what<br>
we have for the i2c or spi would be good.<br>
<a href="https://docs.rtems.org/doxygen/branches/master/group__I2C.html" rel="noreferrer" target="_blank">https://docs.rtems.org/doxygen/branches/master/group__I2C.html</a><br>
<br>
In addition to the shell command addition, can you create/extend a<br>
testsuite for exercising the API?<br>
<br>
I will address the code directly also, but these high-level additions<br>
will greatly improve the submission of a new API. Unfortunately, my<br>
work on code formatting is not quite complete enough to rely on it for<br>
fixing any style issues. I'll do what I can to guide you through that.<br></blockquote><div><br></div><div><div>Aaron,</div><div>Just in case you haven't already found it, the document you should be using for style and 
header documentation is here: 
<a href="https://docs.rtems.org/branches/master/eng/coding-conventions.html">https://docs.rtems.org/branches/master/eng/coding-conventions.html</a></div><div><br></div><div>Kinsey</div> </div></div></div>