On Tue, Nov 9, 2021 at 8:26 AM Martinez, Ricardo wrote:

On 11/6/2021 11:10 AM, Sergey Ryazanov wrote:

quoted

A one nitpick that is common for the entire series. Please consider
using a common prefix for all driver function names (e.g. t7xx_) to
make them more specific. This should improve the code readability.
Thus, any reader will know for sure that the called functions belong
to the driver, and not to a generic kernel API. E.g. use the
t7xx_cldma_hw_init() name for the  CLDMA initialization function
instead of the too generic cldma_hw_init() name, etc.

Does this apply to static functions as well?

As I wrote, this is a nitpick. As you can see in
Documentation/process/coding-style.rst, there are no general rules for
functions naming. My personal rule of thumb is that if  a function
performs a very general operation (like averaging, interpolation,
etc.), then a prefix can be omitted. If a function operation is
specific for a module, then add a common module prefix to the function
name. But again, this is my personal rule.

As for the driver, it was quite difficult to read the code that calls
functions such as cldma_alloc(), cldma_init(). It was hard to figure
out whether these functions are new kernel API or they are specific to
the driver. A common way to solve such ambiguity issues is to prefix
the driver function names. But again, this was just an attempt to draw
your attention to the function naming. Feel free to name functions as
you would like, just make the code readable for developers who are not
familiar with the specific HW chip.

quoted

Another common drawback is that the driver should break as soon as two
modems are connected simultaneously. This should happen due to the use
of multiple _global_ variables that keeps pointers to a modem runtime
state. Out of curiosity, did you test the driver with two or more
modems connected simultaneously?

We haven't tested such configurations, we are focusing on platforms with one single modem.

Now you are aware of the potential kernel crash due to the global
variables misuse. Please fix it.

-- 
Sergey