Date:   Wed, 1 Feb 2023 12:54:02 -0600
From:   Rob Herring <robh@kernel.org>
To:     Michael Walle <michael@walle.cc>
Cc:     =?utf-8?B?UmFmYcWCIE1pxYJlY2tp?= <zajec5@gmail.com>,
        devicetree@vger.kernel.org, hayashi.kunihiko@socionext.com,
        krzysztof.kozlowski+dt@linaro.org,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
        linux-mediatek@lists.infradead.org, matthias.bgg@gmail.com,
        mhiramat@kernel.org, rafal@milecki.pl,
        srinivas.kandagatla@linaro.org
Subject: Re: [PATCH 3/4] nvmem: mtk-efuse: replace driver with a generic MMIO
 one
Message-ID: <20230201185402.GA4084724-robh@kernel.org>
References: <20230201064717.18410-4-zajec5@gmail.com>
 <20230201084821.1719839-1-michael@walle.cc>
 <8452b341-8695-05d8-9d03-47c9aeca0ec7@gmail.com>
 <017a17eb99ac2b2c858d27b65c5dd372@walle.cc>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <017a17eb99ac2b2c858d27b65c5dd372@walle.cc>
Precedence: bulk

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.

Personally, I think it is easy enough to revive the driver if needed 
unless writing is a soon and likely feature.

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.

Rob