On 2/20/23 22:29, Ulf Samuelsson wrote:
[...]
To sum it up:
- The only part of the MTD subsystem that is in use by this driver is
the write callback, everything else is unused. That does not make MTD
subsystem the right tool.
No, the MTD subsystem is compatible with the device tree.
That does not justify overloading the subsystem with unrelated hardware
support for which there is an existing subsystem with existing drivers
for the functionality you would duplicate in the MTD subsystem. There
are existing DT bindings for FPGAs, see Linux
Documentation/devicetree/bindings/fpga/ , use those and add support for
them into the FPGA subsystem if needed .
You can also get information about the FPGA when you list the mtd devices.
No, you cannot, because the information you may get out of the FPGA in
some cases does not map to MTD devices .
It allows you to support partial reconfiguration through the partition
feature. You can write any size block to any part of the MTD, and you
can express that in a device tree. That is what is needed.
I am afraid it is not as simple as that, see Linux
Documentation/devicetree/bindings/fpga/fpga-region.txt . Furthermore,
those DT bindings are already part of Linux and Linux expects the DT to
contain exactly those bindings, U-Boot tries not to diverge from what
Linux does in DT.
- There is existing FPGA subsystem both in U-Boot and Linux, extend it
if necessary. That also covers the usecases which go past a simple
full bitstream upload.
And for use, that introduces a command set, which we do not need
and adds 50-100kB to the image.
If you were to turn everything that needs read/write callback into an
MTD device, you would be able to save even more space, but that does not
make it right. You could try and reduce the FPGA command size if there
is code which is unused for some usecases, that can be selected via
Kconfig at build time.
Obviously, someone would have to take the time to do it, but it is
certainly possible.
You could make the exact same argument about making the FPGA driver a
MISC device, CHAR device, BLOCK device, they all have the write
callback, but that does not mean it is the right fit.
That is because you see the "write" as the only function.
Yes, because that is the only callback this patch implements, the rest
are empty.
[...]
What would work is to keep the driver as is, replacing
the MTD stuff internally with an fpga_add after generating a struct
based on the device tree.
Then it would be part of the FPGA subsystem.
To add this as an Altera passive serial is high risk,
since it cannot be tested on boards using that.
It would simplify things a lot to add it as another driver.
No, that would make things MUCH worse, because we would have a one-off
driver used by one system, therefore poorly tested, which duplicates
functionality of existing driver used by multiple systems and better
tested. This also increases maintenance burden.
One single driver for this common functionality is always better as it
gets more testing.
The result would of course be something which adds both complexity and
code size for us, without any benefit.
Please also consider the MTD maintainer point of view.
[...]
