Re: [RFC PATCH v6 3/3] drivers: nvmem: Add Vybrid OCOTP support
Hello Stefan, On 15-07-08 22:55:23, Stefan Wahren wrote: > Hi Sanchayan, > > > maitysancha...@gmail.com hat am 8. Juli 2015 um 07:39 geschrieben: > > > > > > [...] > > > > > > > > > > Looking at valid_fuse_addr shows me 0x3F as last valid register. So > > > > > the > > > > > rest > > > > > of the buffer ( 0xD00 - sizeof(valid_fuse_addr) ) in case of raw > > > > > access > > > > > could be > > > > > uninitializied. > > > > > > > > Sorry I did not exactly get you here. The intention behind using the > > > > valid_fuse_addr > > > > is to allow reading only from valid fuse addresses and avoid reading > > > > from > > > > all > > > > other > > > > locations as per the Fuse map address table 35-1. > > > > > > Yes, i got your intention. But from my unterstand the read function should > > > fill > > > up > > > the complete buffer with defined values. My MXS OCOTP driver have the same > > > problem > > > and fill up the invalid registers with zero. > > > > I must admit I had not thought of that. Thats a valid point. I dont know at > > the > > moment however what would be the correct approach here. Perhaps filling > > locations > > other than the valid fuse addresses as per the fuse map table with > > 0xBADABADA > > is > > the right thing? Anyways that's what is going to be returned if we were to > > read > > a location which is read locked or reserved. > > since we are operating in userspace the behavior shouldn't be specific to the > device. > It would be good when all drivers behave the same. But i think it would be > much > better > as the nvmem framework take care of it and preinit the buffer with a defined > value. There is a v7 now. Yet to give that a try or look at it. > > > > > However since the fuse address and base address mapping is not exactly > > linear, > > this would require some additional logic than the simple thing I did. > > I defined a regmap_access_table for valid read registers in my case. But i > think > in your case > an implementation of the readable_reg callback in the regmap_config would be > more elegant. > > You can validate your implementation under /sys/kernel/debug/regmap/ Thank you for the input. I will have a look at it and give it a try. > > Sorry, i'm a newbie to regmap. Same here as well :) Regards, Sanchayan. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH v6 3/3] drivers: nvmem: Add Vybrid OCOTP support
Hello Stefan, On 15-07-08 22:55:23, Stefan Wahren wrote: Hi Sanchayan, maitysancha...@gmail.com hat am 8. Juli 2015 um 07:39 geschrieben: [...] Looking at valid_fuse_addr shows me 0x3F as last valid register. So the rest of the buffer ( 0xD00 - sizeof(valid_fuse_addr) ) in case of raw access could be uninitializied. Sorry I did not exactly get you here. The intention behind using the valid_fuse_addr is to allow reading only from valid fuse addresses and avoid reading from all other locations as per the Fuse map address table 35-1. Yes, i got your intention. But from my unterstand the read function should fill up the complete buffer with defined values. My MXS OCOTP driver have the same problem and fill up the invalid registers with zero. I must admit I had not thought of that. Thats a valid point. I dont know at the moment however what would be the correct approach here. Perhaps filling locations other than the valid fuse addresses as per the fuse map table with 0xBADABADA is the right thing? Anyways that's what is going to be returned if we were to read a location which is read locked or reserved. since we are operating in userspace the behavior shouldn't be specific to the device. It would be good when all drivers behave the same. But i think it would be much better as the nvmem framework take care of it and preinit the buffer with a defined value. There is a v7 now. Yet to give that a try or look at it. However since the fuse address and base address mapping is not exactly linear, this would require some additional logic than the simple thing I did. I defined a regmap_access_table for valid read registers in my case. But i think in your case an implementation of the readable_reg callback in the regmap_config would be more elegant. You can validate your implementation under /sys/kernel/debug/regmap/ Thank you for the input. I will have a look at it and give it a try. Sorry, i'm a newbie to regmap. Same here as well :) Regards, Sanchayan. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH v6 3/3] drivers: nvmem: Add Vybrid OCOTP support
Hi Sanchayan, > maitysancha...@gmail.com hat am 8. Juli 2015 um 07:39 geschrieben: > > > [...] > > > > > > > > Looking at valid_fuse_addr shows me 0x3F as last valid register. So the > > > > rest > > > > of the buffer ( 0xD00 - sizeof(valid_fuse_addr) ) in case of raw access > > > > could be > > > > uninitializied. > > > > > > Sorry I did not exactly get you here. The intention behind using the > > > valid_fuse_addr > > > is to allow reading only from valid fuse addresses and avoid reading from > > > all > > > other > > > locations as per the Fuse map address table 35-1. > > > > Yes, i got your intention. But from my unterstand the read function should > > fill > > up > > the complete buffer with defined values. My MXS OCOTP driver have the same > > problem > > and fill up the invalid registers with zero. > > I must admit I had not thought of that. Thats a valid point. I dont know at > the > moment however what would be the correct approach here. Perhaps filling > locations > other than the valid fuse addresses as per the fuse map table with 0xBADABADA > is > the right thing? Anyways that's what is going to be returned if we were to > read > a location which is read locked or reserved. since we are operating in userspace the behavior shouldn't be specific to the device. It would be good when all drivers behave the same. But i think it would be much better as the nvmem framework take care of it and preinit the buffer with a defined value. > > However since the fuse address and base address mapping is not exactly linear, > this would require some additional logic than the simple thing I did. I defined a regmap_access_table for valid read registers in my case. But i think in your case an implementation of the readable_reg callback in the regmap_config would be more elegant. You can validate your implementation under /sys/kernel/debug/regmap/ Sorry, i'm a newbie to regmap. Stefan > Also OCOTP requires the fuse address to be written. May be using the nvmem > consumer DT property > gets something?! Not sure atm. > > - Sanchayan. > > ___ > linux-arm-kernel mailing list > linux-arm-ker...@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH v6 3/3] drivers: nvmem: Add Vybrid OCOTP support
Hi Sanchayan, maitysancha...@gmail.com hat am 8. Juli 2015 um 07:39 geschrieben: [...] Looking at valid_fuse_addr shows me 0x3F as last valid register. So the rest of the buffer ( 0xD00 - sizeof(valid_fuse_addr) ) in case of raw access could be uninitializied. Sorry I did not exactly get you here. The intention behind using the valid_fuse_addr is to allow reading only from valid fuse addresses and avoid reading from all other locations as per the Fuse map address table 35-1. Yes, i got your intention. But from my unterstand the read function should fill up the complete buffer with defined values. My MXS OCOTP driver have the same problem and fill up the invalid registers with zero. I must admit I had not thought of that. Thats a valid point. I dont know at the moment however what would be the correct approach here. Perhaps filling locations other than the valid fuse addresses as per the fuse map table with 0xBADABADA is the right thing? Anyways that's what is going to be returned if we were to read a location which is read locked or reserved. since we are operating in userspace the behavior shouldn't be specific to the device. It would be good when all drivers behave the same. But i think it would be much better as the nvmem framework take care of it and preinit the buffer with a defined value. However since the fuse address and base address mapping is not exactly linear, this would require some additional logic than the simple thing I did. I defined a regmap_access_table for valid read registers in my case. But i think in your case an implementation of the readable_reg callback in the regmap_config would be more elegant. You can validate your implementation under /sys/kernel/debug/regmap/ Sorry, i'm a newbie to regmap. Stefan Also OCOTP requires the fuse address to be written. May be using the nvmem consumer DT property gets something?! Not sure atm. - Sanchayan. ___ linux-arm-kernel mailing list linux-arm-ker...@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH v6 3/3] drivers: nvmem: Add Vybrid OCOTP support
Hi Stefan,
On 15-07-07 14:49:06, Stefan Wahren wrote:
> Hi Sanchayan,
>
> > maitysancha...@gmail.com hat am 7. Juli 2015 um 07:19 geschrieben:
> >
> >
> > [...]
> > > > diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig
> > > > index 17f1a57..84c830d 100644
> > > > --- a/drivers/nvmem/Kconfig
> > > > +++ b/drivers/nvmem/Kconfig
> > > > @@ -33,4 +33,14 @@ config NVMEM_SUNXI_SID
> > > > This driver can also be built as a module. If so, the module
> > > > will be called eeprom-sunxi-sid.
> > > >
> > > > +config NVMEM_VF610_OCOTP
> > > > + tristate "VF610 SoCs OCOTP support"
> > > > + depends on SOC_VF610
> > > > + help
> > > > + This is a driver for the 'OCOTP' available on various Vybrid
> > > > + devices.
> > >
> > > I don't know much about Vybrid. But this driver is specific for VF610,
> > > isn't
> > > it?
> >
> > Sorry. I only checked on VF50 and VF61. Will check if is it available with
> > the
> > other Vybrid devices and change if it is not so.
>
> no problem. If you spend time in testing your driver with different devices,
> you
> could mention
> this in your patch description. The naming VF610 suggests that the driver is
> very specific.
> That confuses me a little bit.
Sorry about that. Will add the necessary information in the next respin for
sure.
>
> >
> > >
> > > > +
> > > > +
> > > > +static int vf610_ocotp_write(void *context, const void *data, size_t
> > > > count)
> > > > +{
> > > > + return 0;
> > > > +}
> > > > +
> > > > +static int vf610_ocotp_read(void *context,
> > > > + const void *offset, size_t reg_size,
> > > > + void *val, size_t val_size)
> > > > +{
> > > > + void __iomem *ocotp_base = context;
> > > > + u32 *buf = val;
> > > > + u32 reg;
> > > > + int ret;
> > > > + int i;
> > > > +
> > > > + for (i = 0; i < ARRAY_SIZE(valid_fuse_addr); i++) {
> > > > + vf610_ocotp_set_timing(ocotp_base, ocotp_timing);
> > > > + ret = vf610_ocotp_wait_busy(ocotp_base + OCOTP_CTRL_REG);
> > > > + if (ret)
> > > > + return ret;
> > >
> > > Is it really necessary to set the timing in the loop, instead before?
> >
> > I will test it once. From my understanding of 35.3.1.5 I thought the timing
> > needs to explicitly programmed on every read. Perhaps I took it too
> > literally.
>
> It was only a question. If barebox does the same, it should be okay.
>
Yes, the code logic is completely similar to barebox except for changes like
how timeout is handled here in the wait busy function.
> >
> > >
> > > > +
> > > > + reg = readl(ocotp_base + OCOTP_CTRL_REG);
> > > > + reg &= ~OCOTP_CTRL_ADDR_MASK;
> > > > + reg &= ~OCOTP_CTRL_WR_UNLOCK_MASK;
> > > > + reg |= BF(valid_fuse_addr[i], OCOTP_CTRL_ADDR);
> > > > + writel(reg, ocotp_base + OCOTP_CTRL_REG);
> > > > +
> > > > + writel(OCOTP_READ_CTRL_READ_FUSE,
> > > > + ocotp_base + OCOTP_READ_CTRL_REG);
> > > > + ret = vf610_ocotp_wait_busy(ocotp_base + OCOTP_CTRL_REG);
> > > > + if (ret)
> > > > + return ret;
> > > > +
> > > > + if (readl(ocotp_base) & OCOTP_CTRL_ERROR) {
> > > > + pr_err("Error reading from fuse address %d\n",
> > > > + valid_fuse_addr[i]);
> > >
> > > You could use dev_err() when storing vf610_ocotp_dev in the context.
> >
> > Ok.
> >
> > >
> > > > + writel(OCOTP_CTRL_ERROR, ocotp_base + OCOTP_CTRL_CLR);
> > >
> > > Shouldn't the function abort here?
> >
> > I am not sure about what usage I should follow here. I went for an
> > explicit error message and since 0xBADABADA is expected to be returned
> > on a read locked shadow register, the user would get the above for this
> > particular fuse address and the rest can still be valid since the TRM
> > mentions "subsequent reads to unlocked shadow locations will still work
> > successfully." So I did not abort on the error. Should I?
>
> In case you don't want to abort, a comment about the 0xBADABADA behavior would
> be helpful.
Ok. Will add the same.
>
> >
> > >
> > > > +
> > > > +
> > > > +static int vf610_ocotp_probe(struct platform_device *pdev)
> > > > +{
> > > > + struct vf610_ocotp_dev *ocotp_dev;
> > > > +
> > > > + ocotp_dev = devm_kzalloc(>dev,
> > > > + sizeof(struct vf610_ocotp_dev), GFP_KERNEL);
> > > > + if (!ocotp_dev)
> > > > + return -ENOMEM;
> > > > +
> > > > + ocotp_dev->dev = >dev;
> > > > +
> > > > + ocotp_dev->res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > > > + ocotp_dev->base = devm_ioremap_resource(ocotp_dev->dev,
> > > > ocotp_dev->res);
> > > > + if (IS_ERR(ocotp_dev->base))
> > > > + return PTR_ERR(ocotp_dev->base);
> > > > +
> > > > + ocotp_dev->clk = devm_clk_get(ocotp_dev->dev, "ocotp");
> > > > + if (IS_ERR(ocotp_dev->clk)) {
> > > > + dev_err(ocotp_dev->dev, "failed getting clock, err = %ld\n",
> > > > + PTR_ERR(ocotp_dev->clk));
> > > > + return PTR_ERR(ocotp_dev->clk);
> > > > + }
> > > > +
> > > > + ocotp_regmap_config.max_register = resource_size(ocotp_dev->res) - 1;
> > >
> > > Looking at valid_fuse_addr shows me 0x3F as last valid register. So the
> > > rest
> > > of the buffer ( 0xD00 - sizeof(valid_fuse_addr) ) in case
Re: [RFC PATCH v6 3/3] drivers: nvmem: Add Vybrid OCOTP support
Hi Sanchayan,
> maitysancha...@gmail.com hat am 7. Juli 2015 um 07:19 geschrieben:
>
>
> [...]
> > > diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig
> > > index 17f1a57..84c830d 100644
> > > --- a/drivers/nvmem/Kconfig
> > > +++ b/drivers/nvmem/Kconfig
> > > @@ -33,4 +33,14 @@ config NVMEM_SUNXI_SID
> > > This driver can also be built as a module. If so, the module
> > > will be called eeprom-sunxi-sid.
> > >
> > > +config NVMEM_VF610_OCOTP
> > > + tristate "VF610 SoCs OCOTP support"
> > > + depends on SOC_VF610
> > > + help
> > > + This is a driver for the 'OCOTP' available on various Vybrid
> > > + devices.
> >
> > I don't know much about Vybrid. But this driver is specific for VF610, isn't
> > it?
>
> Sorry. I only checked on VF50 and VF61. Will check if is it available with the
> other Vybrid devices and change if it is not so.
no problem. If you spend time in testing your driver with different devices, you
could mention
this in your patch description. The naming VF610 suggests that the driver is
very specific.
That confuses me a little bit.
>
> >
> > > +
> > > +
> > > +static int vf610_ocotp_write(void *context, const void *data, size_t
> > > count)
> > > +{
> > > + return 0;
> > > +}
> > > +
> > > +static int vf610_ocotp_read(void *context,
> > > + const void *offset, size_t reg_size,
> > > + void *val, size_t val_size)
> > > +{
> > > + void __iomem *ocotp_base = context;
> > > + u32 *buf = val;
> > > + u32 reg;
> > > + int ret;
> > > + int i;
> > > +
> > > + for (i = 0; i < ARRAY_SIZE(valid_fuse_addr); i++) {
> > > + vf610_ocotp_set_timing(ocotp_base, ocotp_timing);
> > > + ret = vf610_ocotp_wait_busy(ocotp_base + OCOTP_CTRL_REG);
> > > + if (ret)
> > > + return ret;
> >
> > Is it really necessary to set the timing in the loop, instead before?
>
> I will test it once. From my understanding of 35.3.1.5 I thought the timing
> needs to explicitly programmed on every read. Perhaps I took it too literally.
It was only a question. If barebox does the same, it should be okay.
>
> >
> > > +
> > > + reg = readl(ocotp_base + OCOTP_CTRL_REG);
> > > + reg &= ~OCOTP_CTRL_ADDR_MASK;
> > > + reg &= ~OCOTP_CTRL_WR_UNLOCK_MASK;
> > > + reg |= BF(valid_fuse_addr[i], OCOTP_CTRL_ADDR);
> > > + writel(reg, ocotp_base + OCOTP_CTRL_REG);
> > > +
> > > + writel(OCOTP_READ_CTRL_READ_FUSE,
> > > + ocotp_base + OCOTP_READ_CTRL_REG);
> > > + ret = vf610_ocotp_wait_busy(ocotp_base + OCOTP_CTRL_REG);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + if (readl(ocotp_base) & OCOTP_CTRL_ERROR) {
> > > + pr_err("Error reading from fuse address %d\n",
> > > + valid_fuse_addr[i]);
> >
> > You could use dev_err() when storing vf610_ocotp_dev in the context.
>
> Ok.
>
> >
> > > + writel(OCOTP_CTRL_ERROR, ocotp_base + OCOTP_CTRL_CLR);
> >
> > Shouldn't the function abort here?
>
> I am not sure about what usage I should follow here. I went for an
> explicit error message and since 0xBADABADA is expected to be returned
> on a read locked shadow register, the user would get the above for this
> particular fuse address and the rest can still be valid since the TRM
> mentions "subsequent reads to unlocked shadow locations will still work
> successfully." So I did not abort on the error. Should I?
In case you don't want to abort, a comment about the 0xBADABADA behavior would
be helpful.
>
> >
> > > +
> > > +
> > > +static int vf610_ocotp_probe(struct platform_device *pdev)
> > > +{
> > > + struct vf610_ocotp_dev *ocotp_dev;
> > > +
> > > + ocotp_dev = devm_kzalloc(>dev,
> > > + sizeof(struct vf610_ocotp_dev), GFP_KERNEL);
> > > + if (!ocotp_dev)
> > > + return -ENOMEM;
> > > +
> > > + ocotp_dev->dev = >dev;
> > > +
> > > + ocotp_dev->res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > > + ocotp_dev->base = devm_ioremap_resource(ocotp_dev->dev, ocotp_dev->res);
> > > + if (IS_ERR(ocotp_dev->base))
> > > + return PTR_ERR(ocotp_dev->base);
> > > +
> > > + ocotp_dev->clk = devm_clk_get(ocotp_dev->dev, "ocotp");
> > > + if (IS_ERR(ocotp_dev->clk)) {
> > > + dev_err(ocotp_dev->dev, "failed getting clock, err = %ld\n",
> > > + PTR_ERR(ocotp_dev->clk));
> > > + return PTR_ERR(ocotp_dev->clk);
> > > + }
> > > +
> > > + ocotp_regmap_config.max_register = resource_size(ocotp_dev->res) - 1;
> >
> > Looking at valid_fuse_addr shows me 0x3F as last valid register. So the rest
> > of the buffer ( 0xD00 - sizeof(valid_fuse_addr) ) in case of raw access
> > could be
> > uninitializied.
>
> Sorry I did not exactly get you here. The intention behind using the
> valid_fuse_addr
> is to allow reading only from valid fuse addresses and avoid reading from all
> other
> locations as per the Fuse map address table 35-1.
Yes, i got your intention. But from my unterstand the read function should fill
up
the complete buffer with defined values. My MXS OCOTP driver have the same
problem
and fill up the invalid registers with zero.
Regards
Stefan
>
> Thanks for the review.
>
> Regards,
> Sanchayan.
--
To unsubscribe from this list: send the
Re: [RFC PATCH v6 3/3] drivers: nvmem: Add Vybrid OCOTP support
Hi Sanchayan,
maitysancha...@gmail.com hat am 7. Juli 2015 um 07:19 geschrieben:
[...]
diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig
index 17f1a57..84c830d 100644
--- a/drivers/nvmem/Kconfig
+++ b/drivers/nvmem/Kconfig
@@ -33,4 +33,14 @@ config NVMEM_SUNXI_SID
This driver can also be built as a module. If so, the module
will be called eeprom-sunxi-sid.
+config NVMEM_VF610_OCOTP
+ tristate VF610 SoCs OCOTP support
+ depends on SOC_VF610
+ help
+ This is a driver for the 'OCOTP' available on various Vybrid
+ devices.
I don't know much about Vybrid. But this driver is specific for VF610, isn't
it?
Sorry. I only checked on VF50 and VF61. Will check if is it available with the
other Vybrid devices and change if it is not so.
no problem. If you spend time in testing your driver with different devices, you
could mention
this in your patch description. The naming VF610 suggests that the driver is
very specific.
That confuses me a little bit.
+
+
+static int vf610_ocotp_write(void *context, const void *data, size_t
count)
+{
+ return 0;
+}
+
+static int vf610_ocotp_read(void *context,
+ const void *offset, size_t reg_size,
+ void *val, size_t val_size)
+{
+ void __iomem *ocotp_base = context;
+ u32 *buf = val;
+ u32 reg;
+ int ret;
+ int i;
+
+ for (i = 0; i ARRAY_SIZE(valid_fuse_addr); i++) {
+ vf610_ocotp_set_timing(ocotp_base, ocotp_timing);
+ ret = vf610_ocotp_wait_busy(ocotp_base + OCOTP_CTRL_REG);
+ if (ret)
+ return ret;
Is it really necessary to set the timing in the loop, instead before?
I will test it once. From my understanding of 35.3.1.5 I thought the timing
needs to explicitly programmed on every read. Perhaps I took it too literally.
It was only a question. If barebox does the same, it should be okay.
+
+ reg = readl(ocotp_base + OCOTP_CTRL_REG);
+ reg = ~OCOTP_CTRL_ADDR_MASK;
+ reg = ~OCOTP_CTRL_WR_UNLOCK_MASK;
+ reg |= BF(valid_fuse_addr[i], OCOTP_CTRL_ADDR);
+ writel(reg, ocotp_base + OCOTP_CTRL_REG);
+
+ writel(OCOTP_READ_CTRL_READ_FUSE,
+ ocotp_base + OCOTP_READ_CTRL_REG);
+ ret = vf610_ocotp_wait_busy(ocotp_base + OCOTP_CTRL_REG);
+ if (ret)
+ return ret;
+
+ if (readl(ocotp_base) OCOTP_CTRL_ERROR) {
+ pr_err(Error reading from fuse address %d\n,
+ valid_fuse_addr[i]);
You could use dev_err() when storing vf610_ocotp_dev in the context.
Ok.
+ writel(OCOTP_CTRL_ERROR, ocotp_base + OCOTP_CTRL_CLR);
Shouldn't the function abort here?
I am not sure about what usage I should follow here. I went for an
explicit error message and since 0xBADABADA is expected to be returned
on a read locked shadow register, the user would get the above for this
particular fuse address and the rest can still be valid since the TRM
mentions subsequent reads to unlocked shadow locations will still work
successfully. So I did not abort on the error. Should I?
In case you don't want to abort, a comment about the 0xBADABADA behavior would
be helpful.
+
+
+static int vf610_ocotp_probe(struct platform_device *pdev)
+{
+ struct vf610_ocotp_dev *ocotp_dev;
+
+ ocotp_dev = devm_kzalloc(pdev-dev,
+ sizeof(struct vf610_ocotp_dev), GFP_KERNEL);
+ if (!ocotp_dev)
+ return -ENOMEM;
+
+ ocotp_dev-dev = pdev-dev;
+
+ ocotp_dev-res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ ocotp_dev-base = devm_ioremap_resource(ocotp_dev-dev, ocotp_dev-res);
+ if (IS_ERR(ocotp_dev-base))
+ return PTR_ERR(ocotp_dev-base);
+
+ ocotp_dev-clk = devm_clk_get(ocotp_dev-dev, ocotp);
+ if (IS_ERR(ocotp_dev-clk)) {
+ dev_err(ocotp_dev-dev, failed getting clock, err = %ld\n,
+ PTR_ERR(ocotp_dev-clk));
+ return PTR_ERR(ocotp_dev-clk);
+ }
+
+ ocotp_regmap_config.max_register = resource_size(ocotp_dev-res) - 1;
Looking at valid_fuse_addr shows me 0x3F as last valid register. So the rest
of the buffer ( 0xD00 - sizeof(valid_fuse_addr) ) in case of raw access
could be
uninitializied.
Sorry I did not exactly get you here. The intention behind using the
valid_fuse_addr
is to allow reading only from valid fuse addresses and avoid reading from all
other
locations as per the Fuse map address table 35-1.
Yes, i got your intention. But from my unterstand the read function should fill
up
the complete buffer with defined values. My MXS OCOTP driver have the same
problem
and fill up the invalid registers with zero.
Regards
Stefan
Thanks for the review.
Regards,
Sanchayan.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH v6 3/3] drivers: nvmem: Add Vybrid OCOTP support
Hi Stefan,
On 15-07-07 14:49:06, Stefan Wahren wrote:
Hi Sanchayan,
maitysancha...@gmail.com hat am 7. Juli 2015 um 07:19 geschrieben:
[...]
diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig
index 17f1a57..84c830d 100644
--- a/drivers/nvmem/Kconfig
+++ b/drivers/nvmem/Kconfig
@@ -33,4 +33,14 @@ config NVMEM_SUNXI_SID
This driver can also be built as a module. If so, the module
will be called eeprom-sunxi-sid.
+config NVMEM_VF610_OCOTP
+ tristate VF610 SoCs OCOTP support
+ depends on SOC_VF610
+ help
+ This is a driver for the 'OCOTP' available on various Vybrid
+ devices.
I don't know much about Vybrid. But this driver is specific for VF610,
isn't
it?
Sorry. I only checked on VF50 and VF61. Will check if is it available with
the
other Vybrid devices and change if it is not so.
no problem. If you spend time in testing your driver with different devices,
you
could mention
this in your patch description. The naming VF610 suggests that the driver is
very specific.
That confuses me a little bit.
Sorry about that. Will add the necessary information in the next respin for
sure.
+
+
+static int vf610_ocotp_write(void *context, const void *data, size_t
count)
+{
+ return 0;
+}
+
+static int vf610_ocotp_read(void *context,
+ const void *offset, size_t reg_size,
+ void *val, size_t val_size)
+{
+ void __iomem *ocotp_base = context;
+ u32 *buf = val;
+ u32 reg;
+ int ret;
+ int i;
+
+ for (i = 0; i ARRAY_SIZE(valid_fuse_addr); i++) {
+ vf610_ocotp_set_timing(ocotp_base, ocotp_timing);
+ ret = vf610_ocotp_wait_busy(ocotp_base + OCOTP_CTRL_REG);
+ if (ret)
+ return ret;
Is it really necessary to set the timing in the loop, instead before?
I will test it once. From my understanding of 35.3.1.5 I thought the timing
needs to explicitly programmed on every read. Perhaps I took it too
literally.
It was only a question. If barebox does the same, it should be okay.
Yes, the code logic is completely similar to barebox except for changes like
how timeout is handled here in the wait busy function.
+
+ reg = readl(ocotp_base + OCOTP_CTRL_REG);
+ reg = ~OCOTP_CTRL_ADDR_MASK;
+ reg = ~OCOTP_CTRL_WR_UNLOCK_MASK;
+ reg |= BF(valid_fuse_addr[i], OCOTP_CTRL_ADDR);
+ writel(reg, ocotp_base + OCOTP_CTRL_REG);
+
+ writel(OCOTP_READ_CTRL_READ_FUSE,
+ ocotp_base + OCOTP_READ_CTRL_REG);
+ ret = vf610_ocotp_wait_busy(ocotp_base + OCOTP_CTRL_REG);
+ if (ret)
+ return ret;
+
+ if (readl(ocotp_base) OCOTP_CTRL_ERROR) {
+ pr_err(Error reading from fuse address %d\n,
+ valid_fuse_addr[i]);
You could use dev_err() when storing vf610_ocotp_dev in the context.
Ok.
+ writel(OCOTP_CTRL_ERROR, ocotp_base + OCOTP_CTRL_CLR);
Shouldn't the function abort here?
I am not sure about what usage I should follow here. I went for an
explicit error message and since 0xBADABADA is expected to be returned
on a read locked shadow register, the user would get the above for this
particular fuse address and the rest can still be valid since the TRM
mentions subsequent reads to unlocked shadow locations will still work
successfully. So I did not abort on the error. Should I?
In case you don't want to abort, a comment about the 0xBADABADA behavior would
be helpful.
Ok. Will add the same.
+
+
+static int vf610_ocotp_probe(struct platform_device *pdev)
+{
+ struct vf610_ocotp_dev *ocotp_dev;
+
+ ocotp_dev = devm_kzalloc(pdev-dev,
+ sizeof(struct vf610_ocotp_dev), GFP_KERNEL);
+ if (!ocotp_dev)
+ return -ENOMEM;
+
+ ocotp_dev-dev = pdev-dev;
+
+ ocotp_dev-res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ ocotp_dev-base = devm_ioremap_resource(ocotp_dev-dev,
ocotp_dev-res);
+ if (IS_ERR(ocotp_dev-base))
+ return PTR_ERR(ocotp_dev-base);
+
+ ocotp_dev-clk = devm_clk_get(ocotp_dev-dev, ocotp);
+ if (IS_ERR(ocotp_dev-clk)) {
+ dev_err(ocotp_dev-dev, failed getting clock, err = %ld\n,
+ PTR_ERR(ocotp_dev-clk));
+ return PTR_ERR(ocotp_dev-clk);
+ }
+
+ ocotp_regmap_config.max_register = resource_size(ocotp_dev-res) - 1;
Looking at valid_fuse_addr shows me 0x3F as last valid register. So the
rest
of the buffer ( 0xD00 - sizeof(valid_fuse_addr) ) in case of raw access
could be
uninitializied.
Sorry I did not exactly get you here. The intention behind using the
valid_fuse_addr
is to allow reading only from valid fuse addresses and avoid reading from
all
other
locations as per the Fuse map address table 35-1.
Yes, i got your intention. But from my unterstand the read function should
fill
up
the complete buffer with defined values. My MXS OCOTP driver have the same
problem
and fill
Re: [RFC PATCH v6 3/3] drivers: nvmem: Add Vybrid OCOTP support
Hello Stefan,
On 15-07-06 12:16:34, Stefan Wahren wrote:
> Hi Sanchayan,
>
> > Sanchayan Maity hat am 29. Juni 2015 um 13:22
> > geschrieben:
> >
> >
> > The patch adds support for the On Chip One Time Programmable Peripheral
> > (OCOTP) on the Vybrid platform.
>
> please provide a changelog in your cover letter and a new version after
> changes.
>
> I think it's important to note that the driver only support read-only access.
Ok. Will mention this explicitly in the next version.
>
> >
> > Signed-off-by: Sanchayan Maity
> > ---
> > drivers/nvmem/Kconfig | 10 ++
> > drivers/nvmem/Makefile | 2 +
> > drivers/nvmem/vf610-ocotp.c | 250
> >
> > 3 files changed, 262 insertions(+)
> > create mode 100644 drivers/nvmem/vf610-ocotp.c
> >
> > diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig
> > index 17f1a57..84c830d 100644
> > --- a/drivers/nvmem/Kconfig
> > +++ b/drivers/nvmem/Kconfig
> > @@ -33,4 +33,14 @@ config NVMEM_SUNXI_SID
> > This driver can also be built as a module. If so, the module
> > will be called eeprom-sunxi-sid.
> >
> > +config NVMEM_VF610_OCOTP
> > + tristate "VF610 SoCs OCOTP support"
> > + depends on SOC_VF610
> > + help
> > + This is a driver for the 'OCOTP' available on various Vybrid
> > + devices.
>
> I don't know much about Vybrid. But this driver is specific for VF610, isn't
> it?
Sorry. I only checked on VF50 and VF61. Will check if is it available with the
other Vybrid devices and change if it is not so.
>
> > +
> > + This driver can also be built as a module. If so, the module
> > + will be called nvmem-vf610-ocotp.
> > +
> > endif
> > diff --git a/drivers/nvmem/Makefile b/drivers/nvmem/Makefile
> > index cc46791..a9ed113 100644
> > --- a/drivers/nvmem/Makefile
> > +++ b/drivers/nvmem/Makefile
> > @@ -11,3 +11,5 @@ obj-$(CONFIG_QCOM_QFPROM) += nvmem_qfprom.o
> > nvmem_qfprom-y := qfprom.o
> > obj-$(CONFIG_NVMEM_SUNXI_SID) += nvmem-sunxi-sid.o
> > nvmem-sunxi-sid-y := sunxi-sid.o
> > +obj-$(CONFIG_NVMEM_VF610_OCOTP) += nvmem-vf610-ocotp.o
> > +nvmem-vf610-ocotp-y := vf610-ocotp.o
> > diff --git a/drivers/nvmem/vf610-ocotp.c b/drivers/nvmem/vf610-ocotp.c
> > new file mode 100644
> > index 000..b7a782c
> > --- /dev/null
> > +++ b/drivers/nvmem/vf610-ocotp.c
> > @@ -0,0 +1,250 @@
> > +/*
> > + * Copyright (C) 2015 Sanchayan Maity
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 and
> > + * only version 2 as published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + * GNU General Public License for more details.
> > + */
> > +
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +/* OCOTP Register Offsets */
> > +#define OCOTP_CTRL_REG 0x00
> > +#define OCOTP_CTRL_SET 0x04
> > +#define OCOTP_CTRL_CLR 0x08
> > +#define OCOTP_TIMING 0x10
> > +#define OCOTP_DATA 0x20
> > +#define OCOTP_READ_CTRL_REG 0x30
> > +#define OCOTP_READ_FUSE_DATA 0x40
> > +
> > +/* OCOTP Register bits and masks */
> > +#define OCOTP_CTRL_WR_UNLOCK 16
> > +#define OCOTP_CTRL_WR_UNLOCK_KEY 0x3E77
> > +#define OCOTP_CTRL_WR_UNLOCK_MASK 0x
> > +#define OCOTP_CTRL_ADDR 0
> > +#define OCOTP_CTRL_ADDR_MASK 0x7F
> > +#define OCOTP_CTRL_RELOAD_SHADOWS (0x1 << 10)
> > +#define OCOTP_CTRL_ERROR (0x1 << 9)
> > +#define OCOTP_CTRL_BUSY (0x1 << 8)
>
> You can use the BIT() macro here.
Ok.
>
> > +
> > +#define OCOTP_TIMING_STROBE_READ 16
> > +#define OCOTP_TIMING_STROBE_READ_MASK 0x003F
> > +#define OCOTP_TIMING_RELAX 12
> > +#define OCOTP_TIMING_RELAX_MASK 0xF000
> > +#define OCOTP_TIMING_STROBE_PROG 0
> > +#define OCOTP_TIMING_STROBE_PROG_MASK 0x0FFF
> > +
> > +#define OCOTP_READ_CTRL_READ_FUSE 0x1
> > +
> > +#define VF610_OCOTP_TIMEOUT 10
> > +
> > +#define BF(value, field) (((value) << field) & field##_MASK)
> > +
> > +#define DEF_RELAX 20
> > +
> > +struct vf610_ocotp_dev {
> > + void __iomem *base;
> > + struct clk *clk;
> > + struct device *dev;
> > + struct resource *res;
> > + struct regmap *regmap;
> > + struct nvmem_device *nvmem;
> > +};
>
> Some member of this struct seems unnecessary to me. Please
> remove the unused one.
>
> > +
> > +static int ocotp_timing;
>
> How about storing the timings in struct above?
Ok.
>
> > +
> > +static u8 valid_fuse_addr[] = {
> > + 0x00, 0x01, 0x02, 0x04, 0x0F, 0x20, 0x21, 0x22, 0x23, 0x24,
> > + 0x25, 0x26, 0x27, 0x28, 0x2F, 0x38, 0x39, 0x3A, 0x3B, 0x3C,
> > + 0x3D, 0x3E, 0x3F
> > +};
>
> const?
Right.
>
> > +
> > +static int vf610_ocotp_wait_busy(void __iomem *base)
> > +{
> > + int timeout = VF610_OCOTP_TIMEOUT;
> > +
> > + while ((readl(base) &
Re: [RFC PATCH v6 3/3] drivers: nvmem: Add Vybrid OCOTP support
Hi Sanchayan,
> Sanchayan Maity hat am 29. Juni 2015 um 13:22
> geschrieben:
>
>
> The patch adds support for the On Chip One Time Programmable Peripheral
> (OCOTP) on the Vybrid platform.
please provide a changelog in your cover letter and a new version after changes.
I think it's important to note that the driver only support read-only access.
>
> Signed-off-by: Sanchayan Maity
> ---
> drivers/nvmem/Kconfig | 10 ++
> drivers/nvmem/Makefile | 2 +
> drivers/nvmem/vf610-ocotp.c | 250
> 3 files changed, 262 insertions(+)
> create mode 100644 drivers/nvmem/vf610-ocotp.c
>
> diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig
> index 17f1a57..84c830d 100644
> --- a/drivers/nvmem/Kconfig
> +++ b/drivers/nvmem/Kconfig
> @@ -33,4 +33,14 @@ config NVMEM_SUNXI_SID
> This driver can also be built as a module. If so, the module
> will be called eeprom-sunxi-sid.
>
> +config NVMEM_VF610_OCOTP
> + tristate "VF610 SoCs OCOTP support"
> + depends on SOC_VF610
> + help
> + This is a driver for the 'OCOTP' available on various Vybrid
> + devices.
I don't know much about Vybrid. But this driver is specific for VF610, isn't it?
> +
> + This driver can also be built as a module. If so, the module
> + will be called nvmem-vf610-ocotp.
> +
> endif
> diff --git a/drivers/nvmem/Makefile b/drivers/nvmem/Makefile
> index cc46791..a9ed113 100644
> --- a/drivers/nvmem/Makefile
> +++ b/drivers/nvmem/Makefile
> @@ -11,3 +11,5 @@ obj-$(CONFIG_QCOM_QFPROM) += nvmem_qfprom.o
> nvmem_qfprom-y := qfprom.o
> obj-$(CONFIG_NVMEM_SUNXI_SID) += nvmem-sunxi-sid.o
> nvmem-sunxi-sid-y := sunxi-sid.o
> +obj-$(CONFIG_NVMEM_VF610_OCOTP) += nvmem-vf610-ocotp.o
> +nvmem-vf610-ocotp-y := vf610-ocotp.o
> diff --git a/drivers/nvmem/vf610-ocotp.c b/drivers/nvmem/vf610-ocotp.c
> new file mode 100644
> index 000..b7a782c
> --- /dev/null
> +++ b/drivers/nvmem/vf610-ocotp.c
> @@ -0,0 +1,250 @@
> +/*
> + * Copyright (C) 2015 Sanchayan Maity
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 and
> + * only version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +/* OCOTP Register Offsets */
> +#define OCOTP_CTRL_REG 0x00
> +#define OCOTP_CTRL_SET 0x04
> +#define OCOTP_CTRL_CLR 0x08
> +#define OCOTP_TIMING 0x10
> +#define OCOTP_DATA 0x20
> +#define OCOTP_READ_CTRL_REG 0x30
> +#define OCOTP_READ_FUSE_DATA 0x40
> +
> +/* OCOTP Register bits and masks */
> +#define OCOTP_CTRL_WR_UNLOCK 16
> +#define OCOTP_CTRL_WR_UNLOCK_KEY 0x3E77
> +#define OCOTP_CTRL_WR_UNLOCK_MASK 0x
> +#define OCOTP_CTRL_ADDR 0
> +#define OCOTP_CTRL_ADDR_MASK 0x7F
> +#define OCOTP_CTRL_RELOAD_SHADOWS (0x1 << 10)
> +#define OCOTP_CTRL_ERROR (0x1 << 9)
> +#define OCOTP_CTRL_BUSY (0x1 << 8)
You can use the BIT() macro here.
> +
> +#define OCOTP_TIMING_STROBE_READ 16
> +#define OCOTP_TIMING_STROBE_READ_MASK 0x003F
> +#define OCOTP_TIMING_RELAX 12
> +#define OCOTP_TIMING_RELAX_MASK 0xF000
> +#define OCOTP_TIMING_STROBE_PROG 0
> +#define OCOTP_TIMING_STROBE_PROG_MASK 0x0FFF
> +
> +#define OCOTP_READ_CTRL_READ_FUSE 0x1
> +
> +#define VF610_OCOTP_TIMEOUT 10
> +
> +#define BF(value, field) (((value) << field) & field##_MASK)
> +
> +#define DEF_RELAX 20
> +
> +struct vf610_ocotp_dev {
> + void __iomem *base;
> + struct clk *clk;
> + struct device *dev;
> + struct resource *res;
> + struct regmap *regmap;
> + struct nvmem_device *nvmem;
> +};
Some member of this struct seems unnecessary to me. Please
remove the unused one.
> +
> +static int ocotp_timing;
How about storing the timings in struct above?
> +
> +static u8 valid_fuse_addr[] = {
> + 0x00, 0x01, 0x02, 0x04, 0x0F, 0x20, 0x21, 0x22, 0x23, 0x24,
> + 0x25, 0x26, 0x27, 0x28, 0x2F, 0x38, 0x39, 0x3A, 0x3B, 0x3C,
> + 0x3D, 0x3E, 0x3F
> +};
const?
> +
> +static int vf610_ocotp_wait_busy(void __iomem *base)
> +{
> + int timeout = VF610_OCOTP_TIMEOUT;
> +
> + while ((readl(base) & OCOTP_CTRL_BUSY) && --timeout)
> + udelay(10);
> +
> + if (!timeout) {
> + writel(OCOTP_CTRL_ERROR, base + OCOTP_CTRL_CLR);
> + return -ETIMEDOUT;
> + }
> +
> + udelay(10);
> +
> + return 0;
> +}
> +
> +static int vf610_ocotp_calculate_timing(struct vf610_ocotp_dev *ocotp_dev)
> +{
> + u32 clk_rate;
> + u32 relax, strobe_read, strobe_prog;
> + u32 timing;
> +
> + clk_rate = clk_get_rate(ocotp_dev->clk);
If clk_get_rate() fails, then we should abort with a warning.
> +
> + relax = clk_rate / (10 / DEF_RELAX) - 1;
> + strobe_prog = clk_rate / (10 / 1) + 2 * (DEF_RELAX
Re: [RFC PATCH v6 3/3] drivers: nvmem: Add Vybrid OCOTP support
Hi Sanchayan,
Sanchayan Maity maitysancha...@gmail.com hat am 29. Juni 2015 um 13:22
geschrieben:
The patch adds support for the On Chip One Time Programmable Peripheral
(OCOTP) on the Vybrid platform.
please provide a changelog in your cover letter and a new version after changes.
I think it's important to note that the driver only support read-only access.
Signed-off-by: Sanchayan Maity maitysancha...@gmail.com
---
drivers/nvmem/Kconfig | 10 ++
drivers/nvmem/Makefile | 2 +
drivers/nvmem/vf610-ocotp.c | 250
3 files changed, 262 insertions(+)
create mode 100644 drivers/nvmem/vf610-ocotp.c
diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig
index 17f1a57..84c830d 100644
--- a/drivers/nvmem/Kconfig
+++ b/drivers/nvmem/Kconfig
@@ -33,4 +33,14 @@ config NVMEM_SUNXI_SID
This driver can also be built as a module. If so, the module
will be called eeprom-sunxi-sid.
+config NVMEM_VF610_OCOTP
+ tristate VF610 SoCs OCOTP support
+ depends on SOC_VF610
+ help
+ This is a driver for the 'OCOTP' available on various Vybrid
+ devices.
I don't know much about Vybrid. But this driver is specific for VF610, isn't it?
+
+ This driver can also be built as a module. If so, the module
+ will be called nvmem-vf610-ocotp.
+
endif
diff --git a/drivers/nvmem/Makefile b/drivers/nvmem/Makefile
index cc46791..a9ed113 100644
--- a/drivers/nvmem/Makefile
+++ b/drivers/nvmem/Makefile
@@ -11,3 +11,5 @@ obj-$(CONFIG_QCOM_QFPROM) += nvmem_qfprom.o
nvmem_qfprom-y := qfprom.o
obj-$(CONFIG_NVMEM_SUNXI_SID) += nvmem-sunxi-sid.o
nvmem-sunxi-sid-y := sunxi-sid.o
+obj-$(CONFIG_NVMEM_VF610_OCOTP) += nvmem-vf610-ocotp.o
+nvmem-vf610-ocotp-y := vf610-ocotp.o
diff --git a/drivers/nvmem/vf610-ocotp.c b/drivers/nvmem/vf610-ocotp.c
new file mode 100644
index 000..b7a782c
--- /dev/null
+++ b/drivers/nvmem/vf610-ocotp.c
@@ -0,0 +1,250 @@
+/*
+ * Copyright (C) 2015 Sanchayan Maity sanchayan.ma...@toradex.com
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include linux/clk.h
+#include linux/delay.h
+#include linux/device.h
+#include linux/nvmem-provider.h
+#include linux/io.h
+#include linux/module.h
+#include linux/of.h
+#include linux/platform_device.h
+#include linux/regmap.h
+#include linux/slab.h
+
+/* OCOTP Register Offsets */
+#define OCOTP_CTRL_REG 0x00
+#define OCOTP_CTRL_SET 0x04
+#define OCOTP_CTRL_CLR 0x08
+#define OCOTP_TIMING 0x10
+#define OCOTP_DATA 0x20
+#define OCOTP_READ_CTRL_REG 0x30
+#define OCOTP_READ_FUSE_DATA 0x40
+
+/* OCOTP Register bits and masks */
+#define OCOTP_CTRL_WR_UNLOCK 16
+#define OCOTP_CTRL_WR_UNLOCK_KEY 0x3E77
+#define OCOTP_CTRL_WR_UNLOCK_MASK 0x
+#define OCOTP_CTRL_ADDR 0
+#define OCOTP_CTRL_ADDR_MASK 0x7F
+#define OCOTP_CTRL_RELOAD_SHADOWS (0x1 10)
+#define OCOTP_CTRL_ERROR (0x1 9)
+#define OCOTP_CTRL_BUSY (0x1 8)
You can use the BIT() macro here.
+
+#define OCOTP_TIMING_STROBE_READ 16
+#define OCOTP_TIMING_STROBE_READ_MASK 0x003F
+#define OCOTP_TIMING_RELAX 12
+#define OCOTP_TIMING_RELAX_MASK 0xF000
+#define OCOTP_TIMING_STROBE_PROG 0
+#define OCOTP_TIMING_STROBE_PROG_MASK 0x0FFF
+
+#define OCOTP_READ_CTRL_READ_FUSE 0x1
+
+#define VF610_OCOTP_TIMEOUT 10
+
+#define BF(value, field) (((value) field) field##_MASK)
+
+#define DEF_RELAX 20
+
+struct vf610_ocotp_dev {
+ void __iomem *base;
+ struct clk *clk;
+ struct device *dev;
+ struct resource *res;
+ struct regmap *regmap;
+ struct nvmem_device *nvmem;
+};
Some member of this struct seems unnecessary to me. Please
remove the unused one.
+
+static int ocotp_timing;
How about storing the timings in struct above?
+
+static u8 valid_fuse_addr[] = {
+ 0x00, 0x01, 0x02, 0x04, 0x0F, 0x20, 0x21, 0x22, 0x23, 0x24,
+ 0x25, 0x26, 0x27, 0x28, 0x2F, 0x38, 0x39, 0x3A, 0x3B, 0x3C,
+ 0x3D, 0x3E, 0x3F
+};
const?
+
+static int vf610_ocotp_wait_busy(void __iomem *base)
+{
+ int timeout = VF610_OCOTP_TIMEOUT;
+
+ while ((readl(base) OCOTP_CTRL_BUSY) --timeout)
+ udelay(10);
+
+ if (!timeout) {
+ writel(OCOTP_CTRL_ERROR, base + OCOTP_CTRL_CLR);
+ return -ETIMEDOUT;
+ }
+
+ udelay(10);
+
+ return 0;
+}
+
+static int vf610_ocotp_calculate_timing(struct vf610_ocotp_dev *ocotp_dev)
+{
+ u32 clk_rate;
+ u32 relax, strobe_read, strobe_prog;
+ u32 timing;
+
+ clk_rate = clk_get_rate(ocotp_dev-clk);
If clk_get_rate() fails, then we should abort with a warning.
+
+ relax = clk_rate / (10 / DEF_RELAX) - 1;
+ strobe_prog
Re: [RFC PATCH v6 3/3] drivers: nvmem: Add Vybrid OCOTP support
Hello Stefan,
On 15-07-06 12:16:34, Stefan Wahren wrote:
Hi Sanchayan,
Sanchayan Maity maitysancha...@gmail.com hat am 29. Juni 2015 um 13:22
geschrieben:
The patch adds support for the On Chip One Time Programmable Peripheral
(OCOTP) on the Vybrid platform.
please provide a changelog in your cover letter and a new version after
changes.
I think it's important to note that the driver only support read-only access.
Ok. Will mention this explicitly in the next version.
Signed-off-by: Sanchayan Maity maitysancha...@gmail.com
---
drivers/nvmem/Kconfig | 10 ++
drivers/nvmem/Makefile | 2 +
drivers/nvmem/vf610-ocotp.c | 250
3 files changed, 262 insertions(+)
create mode 100644 drivers/nvmem/vf610-ocotp.c
diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig
index 17f1a57..84c830d 100644
--- a/drivers/nvmem/Kconfig
+++ b/drivers/nvmem/Kconfig
@@ -33,4 +33,14 @@ config NVMEM_SUNXI_SID
This driver can also be built as a module. If so, the module
will be called eeprom-sunxi-sid.
+config NVMEM_VF610_OCOTP
+ tristate VF610 SoCs OCOTP support
+ depends on SOC_VF610
+ help
+ This is a driver for the 'OCOTP' available on various Vybrid
+ devices.
I don't know much about Vybrid. But this driver is specific for VF610, isn't
it?
Sorry. I only checked on VF50 and VF61. Will check if is it available with the
other Vybrid devices and change if it is not so.
+
+ This driver can also be built as a module. If so, the module
+ will be called nvmem-vf610-ocotp.
+
endif
diff --git a/drivers/nvmem/Makefile b/drivers/nvmem/Makefile
index cc46791..a9ed113 100644
--- a/drivers/nvmem/Makefile
+++ b/drivers/nvmem/Makefile
@@ -11,3 +11,5 @@ obj-$(CONFIG_QCOM_QFPROM) += nvmem_qfprom.o
nvmem_qfprom-y := qfprom.o
obj-$(CONFIG_NVMEM_SUNXI_SID) += nvmem-sunxi-sid.o
nvmem-sunxi-sid-y := sunxi-sid.o
+obj-$(CONFIG_NVMEM_VF610_OCOTP) += nvmem-vf610-ocotp.o
+nvmem-vf610-ocotp-y := vf610-ocotp.o
diff --git a/drivers/nvmem/vf610-ocotp.c b/drivers/nvmem/vf610-ocotp.c
new file mode 100644
index 000..b7a782c
--- /dev/null
+++ b/drivers/nvmem/vf610-ocotp.c
@@ -0,0 +1,250 @@
+/*
+ * Copyright (C) 2015 Sanchayan Maity sanchayan.ma...@toradex.com
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include linux/clk.h
+#include linux/delay.h
+#include linux/device.h
+#include linux/nvmem-provider.h
+#include linux/io.h
+#include linux/module.h
+#include linux/of.h
+#include linux/platform_device.h
+#include linux/regmap.h
+#include linux/slab.h
+
+/* OCOTP Register Offsets */
+#define OCOTP_CTRL_REG 0x00
+#define OCOTP_CTRL_SET 0x04
+#define OCOTP_CTRL_CLR 0x08
+#define OCOTP_TIMING 0x10
+#define OCOTP_DATA 0x20
+#define OCOTP_READ_CTRL_REG 0x30
+#define OCOTP_READ_FUSE_DATA 0x40
+
+/* OCOTP Register bits and masks */
+#define OCOTP_CTRL_WR_UNLOCK 16
+#define OCOTP_CTRL_WR_UNLOCK_KEY 0x3E77
+#define OCOTP_CTRL_WR_UNLOCK_MASK 0x
+#define OCOTP_CTRL_ADDR 0
+#define OCOTP_CTRL_ADDR_MASK 0x7F
+#define OCOTP_CTRL_RELOAD_SHADOWS (0x1 10)
+#define OCOTP_CTRL_ERROR (0x1 9)
+#define OCOTP_CTRL_BUSY (0x1 8)
You can use the BIT() macro here.
Ok.
+
+#define OCOTP_TIMING_STROBE_READ 16
+#define OCOTP_TIMING_STROBE_READ_MASK 0x003F
+#define OCOTP_TIMING_RELAX 12
+#define OCOTP_TIMING_RELAX_MASK 0xF000
+#define OCOTP_TIMING_STROBE_PROG 0
+#define OCOTP_TIMING_STROBE_PROG_MASK 0x0FFF
+
+#define OCOTP_READ_CTRL_READ_FUSE 0x1
+
+#define VF610_OCOTP_TIMEOUT 10
+
+#define BF(value, field) (((value) field) field##_MASK)
+
+#define DEF_RELAX 20
+
+struct vf610_ocotp_dev {
+ void __iomem *base;
+ struct clk *clk;
+ struct device *dev;
+ struct resource *res;
+ struct regmap *regmap;
+ struct nvmem_device *nvmem;
+};
Some member of this struct seems unnecessary to me. Please
remove the unused one.
+
+static int ocotp_timing;
How about storing the timings in struct above?
Ok.
+
+static u8 valid_fuse_addr[] = {
+ 0x00, 0x01, 0x02, 0x04, 0x0F, 0x20, 0x21, 0x22, 0x23, 0x24,
+ 0x25, 0x26, 0x27, 0x28, 0x2F, 0x38, 0x39, 0x3A, 0x3B, 0x3C,
+ 0x3D, 0x3E, 0x3F
+};
const?
Right.
+
+static int vf610_ocotp_wait_busy(void __iomem *base)
+{
+ int timeout = VF610_OCOTP_TIMEOUT;
+
+ while ((readl(base) OCOTP_CTRL_BUSY) --timeout)
+ udelay(10);
+
+ if (!timeout) {
+
