Am 2023-02-01 19:54, schrieb Rob Herring:
On Wed, Feb 01, 2023 at 11:46:01AM +0100, Michael Walle wrote:
> Before I convert brcm,nvram to NVMEM layout I need some binding & driver
> providing MMIO device access. How to handle that?
I'm not arguing against having the mmio nvmem driver. But I don't
think we should sacrifice possible write access with other drivers.
And
I presume write access won't be possible with your generic driver as
it
probably isn't just a memcpy_toio().
It is a great fallback for some nvmem peripherals which just maps a
memory region, but doesn't replace a proper driver for an nvmem
device.
What bothers me the most isn't the driver change. The driver can be
resurrected once someone will do proper write access, but the generic
"mediatek,efuse" compatible together with the comment above the older
compatible string. These imply that you should use "mediatek,efuse",
but we don't know if all mediatek efuse peripherals will be the
same - esp. for writing which is usually more complex than the
reading.
Because the kernel can't pick the "best" driver when there are multiple
matches, it's all Mediatek platforms use the generic driver or all use
the Mediatek driver.
Isn't that the whole point of having multiple compatible strings?
compatible = "fsl,imx27-mmc", "fsl,imx21-mmc";
The OS might either load the driver for "fsl,imx21-mmc" or one for
"fsl,imx27-mmc", with the latter considered to be the preferred one.
Personally, I think it is easy enough to revive the driver if needed
unless writing is a soon and likely feature.
That what was actually triggered my initial reply. We are planning a
new board with a mediatek SoC and we'll likely need the write support.
But I thought the "mediatek,efuse" was a new compatible with this patch
and the (new!) comment make it looks like these compatible are
deprecated
in favor of "mmio-nvmem". Which would make it impossible to distinguish
between the different efuse peripherals and thus make it impossible to
add write support.
The other way to share is providing library functions for drivers to
use. Then the Mediatek driver can use the generic read functions and
custom write functions.
nitpick btw: why not "nvmem-mmio"?
So it's either:
(1) compatible = "mediatek,mt8173-efuse"
(2) compatible = "mediatek,mt8173-efuse", "mmio-nvmem"
(1) will be supported any anyway for older dts and you need to add
the specific compatibles to the nvmem-mmio driver - or keep the
driver as is.
With (2) you wouldn't need to do that and the kernel can load the
proper driver if available or fall back to the nvmem-mmio one. I'd
even make that one "default y" so it will be available on future
kernels and boards can already make use of the nvmem device even
if there is no proper driver for them.
I'd prefer (2). Dunno what the dt maintainers agree.
No because you are changing the DT. The DT can't change when you want
to
change drivers. This thinking is one reason why 'generic' bindings are
rejected.
There is no change in the DT. Newer bindings will have
compatible = "vendor,ip-block", "mmio-nvmem"
when the ip block is compatible with mmio-nvmem. Otherwise I don't get
why there is a mmio-nvmem compatible at all. Just having
compatible = "mmio-nvmem"
looks wrong as it would just work correctly in some minor cases, i.e.
when write support is just a memcpy_toio() - or we deliberately ignore
any write support. But even then, you always tell people to add specific
compatibles for the case when quirks are needed..
-michael
