linux-kernel - Re: [PATCH v2 0/3] Add the efuse driver on rockchip platform

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <55C0E42E.3060509@linaro.org>
Date:	Tue, 04 Aug 2015 17:11:26 +0100
From:	Srinivas Kandagatla <srinivas.kandagatla@...aro.org>
To:	shunqian.zheng@...il.com, Stefan Wahren <stefan.wahren@...e.com>
CC:	mark.rutland@....com, devicetree@...r.kernel.org,
	linux@....linux.org.uk, heiko@...ech.de, pawel.moll@....com,
	ijc+devicetree@...lion.org.uk, linus.walleij@...aro.org,
	linux-kernel@...r.kernel.org, linux-rockchip@...ts.infradead.org,
	robh+dt@...nel.org, galak@...eaurora.org, matthias.bgg@...il.com,
	Maxime Ripard <maxime.ripard@...e-electrons.com>,
	jay.xu@...k-chips.com, linux-arm-kernel@...ts.infradead.org,
	Caesar Wang <wxt@...k-chips.com>
Subject: Re: [PATCH v2 0/3] Add the efuse driver on rockchip platform

Hi Shunqian,

Sorry for delay in reply, I was on Holidays..

Thanks for testing.

On 31/07/15 10:27, Shunqian Zheng wrote:
>
> 1. Without the following diff, `hexdump
> /sys/bus/nvmem/devices/rockchip-efuse0/nvmem` is wrong with "INVALID
> ARGUMENT":
>
> +++ b/drivers/nvmem/core.c
> @@ -67,7 +67,7 @@ static ssize_t bin_attr_nvmem_read(struct file *filp,
> struct kobject *kobj,
>          int rc;
>
>          /* Stop the user from reading */
> -       if (pos > nvmem->size)
> +       if (pos > nvmem->size - 1)
>                  return 0;

Yes, this should have been something like this
-       if (pos > nvmem->size)
+       if (pos >= nvmem->size)
                 return 0;

We can send a fix on top of v9 once its merged.

>
>          if (pos + count > nvmem->size)
>
> RK3288-efuse has  32 x 8bit regs, in dts "reg = <0xffb40000 0x20>;"
> Here is the message dump from nvmem_device:
> [    2.158314] nvmem:
> [    2.158314]           name (null)
> [    2.158314]           stride 1
> [    2.158314]           word_size 1
> [    2.158314]           ncells 0
> [    2.158314]           id 0
> [    2.158314]           users 0
> [    2.158314]           size 32
> [    2.158314]           read_only 0
>
> Do you think there is a leak or I'm messing up ?
>
> 2. About the read operation, eFuse data can be read during device
> probe() and cached, OR,
>      read from eFuse when needed every time. I prefer the second one but
> then, the clock of eFuse may be
>      gated. So before/after reading I have to enable/disable clk like :
>            devm_clk_get(dev, "hclk_efuse256");
>      The trouble is I can't find a way to get the "dev" hander in :
>            static int rockchip_efuse_read(void *context, const void
> *reg, size_t reg_size, void *val, size_t val_size)
>      I am appreciated if you can give some advice.


May be you should use regmap_init_mmio_clk() instead of 
regmap_init_mmio() it will take care of clks.

>      Or, do you think it's reasonable to add hooks before/after read in
> nvmem/core.c like :
>            +       before_read(dev, ...);
>                     rc = regmap_raw_read(nvmem->regmap, pos, buf, count);
>            +       after_read(dev, ...);
>


> 3. In the /sys/bus/nvmem/devices/rockchip-efuse0/, there are files:
>           /sys/devices/platform/ffb40000.efuse/rockchip-efuse0 # ls
>           nvmem      of_node    power      subsystem  uevent
>      Do you have a plan to add the nvmem consumers to /sys/ in nvmem
> framework?
yes, Am waiting for the framework to be merged, I have plans to add this 
feature.

>      For example,  in dts defined the "cpu_leakage":
>          efuse: efuse@...40000 {
>                  compatible = "rockchip,rk3x-efuse";
>                  reg = <0xffb40000 0x20>;
>                  #address-cells = <1>;
>                  #size-cells = <1>;
>                  clocks = <&cru PCLK_EFUSE256>;
>                  clock-names = "pclk_efuse_256";
>
>                  cpu_leakage: cpu_leakage {
>                          reg = <0x17 0x1>;
>                  };
>          };
>     Then nvmem exposes the "cpu_leakage" file in /sys which can be
> read/write.

--srini
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/