RE: [PATCH net-next] misc: Add Renesas Synchronization Management Unit (SMU) support

Ah, so if this is for a PTP related driver, it should probably be integrated into
the PTP subsystem rather than being a separate class.

I was trying to add these functions to PHC subsystem but was not accepted because the functions
are specific to Renesas device and there is no place for those functions in PHC driver.

quoted

quoted

A pure list of register values seems neither particular portable nor

intuitive.

quoted

quoted

How is a user expected to interpret these, and are you sure that any
driver for this class would have the same interpretation at the same

register index?

quoted

quoted

Yes we need a way to dump register values when remote debugging with

customers.

quoted

And all the Renesas SMU has similar register layout

A sysfs interface is a poor choice for this though -- how can you guarantee
that even future Renesas devices follow the exact same register layout? By
encoding the current hardware generation into the user interface, you would
end up having to emulate this on other chips you want to support later.

If it's only for debugging, best leave it out of the public interface, and only
have it in your own copy of the driver until the bugs are gone, or add a
debugfs interface.

I will drop the sysfs change in the new patch

Unit tests are good, but it's better to have them in the kernel.
Can you add the unit test into the patch then?
We now have the kunit framework for running unit tests.

Our unit test is based on ceedling. But I will definitely look into the kunit and try to
Transfer it to kunit for the next release.

quoted

quoted

This tells me that you got the abstraction the wrong way: the common
files should not need to know anything about the specific

implementations.

quoted

quoted

Instead, these should be in separate modules that call exported
functions from the common code.

I got what you mean. But so far it only supports small set of
functions, which is why I don't feet it is worth the effort to over abstract

things.

Then maybe pick one of the two hardware variants and drop the abstraction
you have. You can then add more features before you add a proper
abstraction layer and then the second driver.

If I come up with a new file and move all the abstraction code there, does that work?