lkml.org 
[lkml]   [2020]   [Feb]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v1 0/3] nvmem: Add support for write-only instances, and clean-up
From
Date


On 24/02/2020 17:41, Nicholas Johnson wrote:
> [Based on Linux v5.6-rc3, does not apply successfully to Linux v5.6-rc2]
>
> Hello all,
>
> I offer the first patch in this series to support write-only instances
> of nvmem. The use-case is the Thunderbolt driver, for which Mika
> Westerberg needs write-only nvmem. Refer to 03cd45d2e219 ("thunderbolt:
> Prevent crash if non-active NVMem file is read").
>

Had a look at the crash trace from the mentioned patch.

Why can not we add a check for reg_read in bin_attr_nvmem_read() before
dereferencing it?

The reason I ask this is because removing read_only is not that simple
as you think.
Firstly because a there is no way to derive this flag by just looking at
read/write callbacks.
Providers are much more generic drivers ex: at24 which can have
read/write interfaces implemented, however read only flag is enforced at
board/platform level config either via device tree property bindings or
a write protection gpio.
Removing this is also going to break the device tree bindings.

only alternative I can see ATM is the mentioned check.

--srini



> The second patch in this series reverts the workaround 03cd45d2e219
> ("thunderbolt: Prevent crash if non-active NVMem file is read") which
> Mika applied in the mean time to prevent a kernel-mode NULL dereference.
> If Mika wants to do this himself or there is some reason not to apply
> this, that is fine, but in my mind, it is a logical progression to apply
> one after the other in the same series.
>
> The third patch in this series removes the .read_only field, because we
> do not have a .write_only field. It only makes sense to have both or
> neither. Having either of them makes it hard to be consistent - what
> happens if a driver were to set both .read_only and .write_only? And
> there is the question of deciding if the nvmem is read-only because of
> the .read_only, or based on if the .reg_read is not NULL. What if they
> disagree? This patch does touch a lot of files, and I will understand if
> you do not wish to apply it. It is optional and does not need to be
> applied with the first two.
>
> Thank you in advance for reviewing these.
>
> Kind regards,
>
> Nicholas Johnson (3):
> nvmem: Add support for write-only instances
> Revert "thunderbolt: Prevent crash if non-active NVMem file is read"
> nvmem: Remove .read_only field from nvmem_config
>
> drivers/misc/eeprom/at24.c | 4 +-
> drivers/misc/eeprom/at25.c | 4 +-
> drivers/misc/eeprom/eeprom_93xx46.c | 4 +-
> drivers/mtd/mtdcore.c | 1 -
> drivers/nvmem/bcm-ocotp.c | 1 -
> drivers/nvmem/core.c | 5 +-
> drivers/nvmem/imx-iim.c | 1 -
> drivers/nvmem/imx-ocotp-scu.c | 1 -
> drivers/nvmem/imx-ocotp.c | 1 -
> drivers/nvmem/lpc18xx_otp.c | 1 -
> drivers/nvmem/meson-mx-efuse.c | 1 -
> drivers/nvmem/nvmem-sysfs.c | 77 ++++++++++++++++++++++++++---
> drivers/nvmem/nvmem.h | 1 -
> drivers/nvmem/rockchip-efuse.c | 1 -
> drivers/nvmem/rockchip-otp.c | 1 -
> drivers/nvmem/sc27xx-efuse.c | 1 -
> drivers/nvmem/sprd-efuse.c | 1 -
> drivers/nvmem/stm32-romem.c | 1 -
> drivers/nvmem/sunxi_sid.c | 1 -
> drivers/nvmem/uniphier-efuse.c | 1 -
> drivers/nvmem/zynqmp_nvmem.c | 1 -
> drivers/soc/tegra/fuse/fuse-tegra.c | 1 -
> drivers/thunderbolt/switch.c | 8 ---
> drivers/w1/slaves/w1_ds250x.c | 1 -
> include/linux/nvmem-provider.h | 2 -
> 25 files changed, 77 insertions(+), 45 deletions(-)
>

\
 
 \ /
  Last update: 2020-02-25 16:01    [W:0.123 / U:0.060 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site