Hi Wolfgang! Today I have sent a new version of the patchset and I hope I have been able to get all feedback from you incorporated. > On 21.01.2019, at 20:25, Wolfgang Grandegger <wg@xxxxxxxxxxxxxx> wrote: > > mcp25xxfd_base.c is fine but then you should also use > > - mcp25xxfd_base.h > - struct mcp25xxfd_base_[priv/info] > - function prefix "mcp25xxfd_base_". > > This would make clear, to what file/interface the functions belong to. > ... > Well, I don't thinks it makes sense to separate the register definitions > into CAN and non-CAN related registers. You have defined all register > and bits... even more than really needed. Therefore I prefer to have it > in a separate file to clearly separate it from the interface related > definitions. > ... > The debugfs part is also a lot of code. Therefore my motivation is to > have it apart. And the statistics should only be incremented if > CONFIG_DEBUG_FS is "y". Therefore you need a macro for that purpose. > ... > But then the prefix should be "MCP25XXFD_CAN_". Well, these are > registers of the MCP2517FD and prefix "MCP25XXFD_<register-name>_" is > just fine. > ... > The prefix "CAN_" is reserved for the CAN network and driver interface. > Especially "CAN_INT", etc. is not OK. If I missed a things, then please be patient with me. Just reorganizing the code into more sources (across multiple patches) and adapting the calling parameters has taken quite some time. The one place where I have deviated from your recomendation are the priv sections for spi and can respectively - I have put those into separate mcp25xxfd_priv.h and mcp25xxfd_can_priv.h together with the corresponding support structures. I have also tried to move out one or two optimizations into separate patches and I have also added additional optimizations. Hence the patchset has grown from 4 to 9 patches. In the hope that I have not introduced many regressions moving from V5 vs V6. Thanks, Martin
