Re: [PATCH v2] iio: adc: vf610: Add IIO buffer support for Vybrid ADC
Hello,
On 15-08-08 19:46:00, Jonathan Cameron wrote:
> On 08/08/15 18:22, maitysancha...@gmail.com wrote:
> > Hello Jonathan,
> >
> > On 15-08-08 15:29:47, Jonathan Cameron wrote:
> >> On 05/08/15 13:10, Sanchayan Maity wrote:
> >>> This patch adds support for IIO buffer to the Vybrid ADC driver.
> >>> IIO triggered buffer infrastructure along with iio sysfs trigger
> >>> is used to leverage continuous sampling support provided by the
> >>> ADC block.
> >> Looking good. Just a couple more bits and pieces inline from me.
> >> One or two of which aren't corrected from Peter's review of v1.
> >>
> >> I will want Fugang Dong's ack / review tag on the final version
> >> before applying it however.
> >
> > Sure.
> >
> >> This driver is some distance form my area of expertise!
> >
> > I doubt :).
> >
> >>
> >> Jonathan
> >>>
> >>> Signed-off-by: Sanchayan Maity
> >>> ---
> >>> drivers/iio/adc/Kconfig | 4 ++
> >>> drivers/iio/adc/vf610_adc.c | 107
> >>> +---
> >>> 2 files changed, 105 insertions(+), 6 deletions(-)
> >>>
> >>> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> >>> index 7c55658..4661241 100644
> >>> --- a/drivers/iio/adc/Kconfig
> >>> +++ b/drivers/iio/adc/Kconfig
> >>> @@ -337,6 +337,10 @@ config TWL6030_GPADC
> >>> config VF610_ADC
> >>> tristate "Freescale vf610 ADC driver"
> >>> depends on OF
> >>> + select IIO_BUFFER
> >>> + select IIO_TRIGGER
> >>> + select IIO_SYSFS_TRIGGER
> >> Unless I missed something there is no dependency on this particular
> >> trigger (it just happens to be the one you've been testing with I guess).
> >> Could be driven from a hardware trigger belonging to another device for
> >> example.
> >
> > Yes right in a way. Right now we do not provide or there is no provision
> > for hardware triggers. On the Vybrid, the Peripheral Delay Block (PDB)
> > does the job of providing support for software and hardware triggers. PDB
> > support is not yet there in Linux however.
> Not supplying a trigger is fine, but any trigger (that allows other devices
> to bind to it) will be fine.
Ok.
> >
> > There is also the question of where the Vybrid PDB driver would belong
> > to? In the TRM it is put in the timers but the kernel has no generic timer
> > framework that I am aware of.
> It's been debated a number of times, but no one has ever written one.
> Not seen anything recently on this topic.. Lars, you seen anything?
> (the blackfin-timer trigger is similar to what you are describing I think).
>
> > After some internal reviews we decided to
> > do a platform driver which provided functions ADC driver could called into.
> Does rather feel like there ought to be at least a standard home for these.
> Might be worth asking the arm-soc guys...
> >
> > I have a patchset ready which provides trigger support using PDB however
> > configuring the PDB properly has proven to be tricky. While it works but
> > not reliably with multiple channels and it would be a while before I get
> > that working and post that patchset. So kind of stalled there and just
> > because of two registers which need to be written with the correct value
> > :). For what it's worth if someone comes across this, some discussion
> > here [1] along with patches. (Note however those are a bit old patches
> > not exactly my new work).
> >
> > Sorry for digressing from the topic. Anyways so the idea was to provide
> > sysfs triggers as default for using this continuous sampling. Later the
> > driver may provide additional triggers. So for now I added the sysfs
> > trigger as a select option so that a user won't have to recompile the
> > kernel for using the buffers with continuous sampling.
> It's a policy decision so should really be left to the distro builders.
> There are lots of possible triggers out there (though sysfs might be
> the most likely one!)
>
> Hence don't put the select there in Kconfig. We should shortly have
> Daniel's update to the patches for the high resolution timer
> which would give another obvious choice for starters.
Alright. Will drop that iio sysfs tigger select from the Kconfig.
>
> I doubt many distros build without the sysfs triggers but with other IIO
> stuff.
>
> >
> >>
> >>> + select IIO_TRIGGERED_BUFFER
> >>> help
> >>> Say yes here to support for Vybrid board analog-to-digital converter.
> >>> Since the IP is used for i.MX6SLX, the driver also support i.MX6SLX.
> >>> diff --git a/drivers/iio/adc/vf610_adc.c b/drivers/iio/adc/vf610_adc.c
> >>> index 23b8fb9..97cb2ed 100644
> >>> --- a/drivers/iio/adc/vf610_adc.c
> >>> +++ b/drivers/iio/adc/vf610_adc.c
> >>> @@ -34,8 +34,11 @@
> >>> #include
> >>>
> >>> #include
> >>> +#include
> >>> #include
> >>> -#include
> >>> +#include
> >>> +#include
> >>> +#include
> >>>
> >>> /* This will be the driver name the kernel reports */
> >>> #define DRIVER_NAME "vf610-adc"
> >>> @@ -170,6 +173,7 @@ struct vf610_adc {
> >>> u32
Re: [PATCH v7 4/4] nvmem: Add DT binding documentation for Vybrid OCOTP driver
Hello,
On 15-08-10 10:18:01, Srinivas Kandagatla wrote:
On 06/08/15 16:27, Sanchayan Maity wrote:
Add the devicetree bindings for the Freescale Vybrid On-Chip
OTP driver.
Signed-off-by: Sanchayan Maity maitysancha...@gmail.com
---
.../devicetree/bindings/nvmem/vf610-ocotp.txt| 20
1 file changed, 20 insertions(+)
create mode 100644 Documentation/devicetree/bindings/nvmem/vf610-ocotp.txt
diff --git a/Documentation/devicetree/bindings/nvmem/vf610-ocotp.txt
b/Documentation/devicetree/bindings/nvmem/vf610-ocotp.txt
new file mode 100644
index 000..5556810
--- /dev/null
+++ b/Documentation/devicetree/bindings/nvmem/vf610-ocotp.txt
@@ -0,0 +1,20 @@
+On-Chip OTP Memory for Freescale Vybrid
+
+Required Properties:
+ compatible:
+ - fsl,vf610-ocotp for VF5xx/VF6xx
+ #address-cells : Should be 1
+ #size-cells : Should be 1
+ reg : Address and length of OTP controller registers
Is there a reason to not add clocks property in to the bindings?
An error on my part. Will fix with the next revision.
- Sanchayan.
+
+Example for Vybrid VF5xx/VF6xx:
+
+ ocotp: ocotp@400a5000 {
+ compatible = fsl,vf610-ocotp;
+ #address-cells = 1;
+ #size-cells = 1;
+ reg = 0x400a5000 0xD00;
+ clocks = clks VF610_CLK_OCOTP;
+ clock-names = ocotp;
+ };
+
--
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: [PATCH v7 3/4] drivers: nvmem: Add Vybrid OCOTP support
Hello,
On 15-08-10 10:17:53, Srinivas Kandagatla wrote:
Hi Sanchayan,
Could you add Greg to the to list so that we can request him to pick
this via his tree.
Will add Greg in cc with the next revision.
Few nits, other than that driver looks good.
On 06/08/15 16:27, Sanchayan Maity wrote:
The patch adds support for the On Chip One Time Programmable Peripheral
(OCOTP) on the Vybrid platform.
Signed-off-by: Sanchayan Maity maitysancha...@gmail.com
---
drivers/nvmem/Kconfig | 10 ++
drivers/nvmem/Makefile | 2 +
drivers/nvmem/vf610-ocotp.c | 297
3 files changed, 309 insertions(+)
create mode 100644 drivers/nvmem/vf610-ocotp.c
diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig
index 0b33014..bfd0c02 100644
--- a/drivers/nvmem/Kconfig
+++ b/drivers/nvmem/Kconfig
@@ -47,4 +47,14 @@ config NVMEM_IMX_OCOTP
This driver can also be built as a module. If so, the module
will be called nvmem-imx-ocotp.
+config NVMEM_VF610_OCOTP
+ tristate VF610_SoCs OCOTP support
+ depends on SOC_VF610
You could also add COMPILE_TEST which will ensure that its compile checked.
Ok.
+ help
+ This is a driver for the 'OCOTP' peripheral available on Vybrid
+ devices like VF5xx and VF6xx.
+
+ 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 b512d77..8a1eea8 100644
--- a/drivers/nvmem/Makefile
+++ b/drivers/nvmem/Makefile
@@ -12,3 +12,5 @@ obj-$(CONFIG_NVMEM_SUNXI_SID) += nvmem_sunxi_sid.o
nvmem_sunxi_sid-y := sunxi_sid.o
obj-$(CONFIG_NVMEM_IMX_OCOTP) += nvmem-imx-ocotp.o
nvmem-imx-ocotp-y := imx-ocotp.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..25ee701
--- /dev/null
+++ b/drivers/nvmem/vf610-ocotp.c
@@ -0,0 +1,297 @@
+/*
+ * Copyright (C) 2015 Toradex AG.
+ *
+ * Author: 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/io.h
+#include linux/module.h
+#include linux/nvmem-provider.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_REG0x30
+#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_ADDR0
+#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)
we can use BIT and GENMASK variants here for most of the defines.
Ok.
+
+#define OCOTP_TIMING_STROBE_READ 16
+#define OCOTP_TIMING_STROBE_READ_MASK 0x003F
+#define OCOTP_TIMING_RELAX 12
+#define OCOTP_TIMING_RELAX_MASK0xF000
+#define OCOTP_TIMING_STROBE_PROG 0
+#define OCOTP_TIMING_STROBE_PROG_MASK 0x0FFF
+
+#define OCOTP_READ_CTRL_READ_FUSE 0x1
+
+#define VF610_OCOTP_TIMEOUT10
+
+#define BF(value, field) (((value) field) field##_MASK)
+
+#define DEF_RELAX 20
+
+static const int base_to_fuse_addr_mappings[][2] = {
+ {0x400, 0x00},
+ {0x410, 0x01},
+ {0x420, 0x02},
+ {0x450, 0x05},
+ {0x4F0, 0x0F},
+ {0x600, 0x20},
+ {0x610, 0x21},
+ {0x620, 0x22},
+ {0x630, 0x23},
+ {0x640, 0x24},
+ {0x650, 0x25},
+ {0x660, 0x26},
+ {0x670, 0x27},
+ {0x6F0,
Re: [PATCH v2] iio: adc: vf610: Add IIO buffer support for Vybrid ADC
Hello,
On 15-08-08 19:46:00, Jonathan Cameron wrote:
On 08/08/15 18:22, maitysancha...@gmail.com wrote:
Hello Jonathan,
On 15-08-08 15:29:47, Jonathan Cameron wrote:
On 05/08/15 13:10, Sanchayan Maity wrote:
This patch adds support for IIO buffer to the Vybrid ADC driver.
IIO triggered buffer infrastructure along with iio sysfs trigger
is used to leverage continuous sampling support provided by the
ADC block.
Looking good. Just a couple more bits and pieces inline from me.
One or two of which aren't corrected from Peter's review of v1.
I will want Fugang Dong's ack / review tag on the final version
before applying it however.
Sure.
This driver is some distance form my area of expertise!
I doubt :).
Jonathan
Signed-off-by: Sanchayan Maity maitysancha...@gmail.com
---
drivers/iio/adc/Kconfig | 4 ++
drivers/iio/adc/vf610_adc.c | 107
+---
2 files changed, 105 insertions(+), 6 deletions(-)
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 7c55658..4661241 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -337,6 +337,10 @@ config TWL6030_GPADC
config VF610_ADC
tristate Freescale vf610 ADC driver
depends on OF
+ select IIO_BUFFER
+ select IIO_TRIGGER
+ select IIO_SYSFS_TRIGGER
Unless I missed something there is no dependency on this particular
trigger (it just happens to be the one you've been testing with I guess).
Could be driven from a hardware trigger belonging to another device for
example.
Yes right in a way. Right now we do not provide or there is no provision
for hardware triggers. On the Vybrid, the Peripheral Delay Block (PDB)
does the job of providing support for software and hardware triggers. PDB
support is not yet there in Linux however.
Not supplying a trigger is fine, but any trigger (that allows other devices
to bind to it) will be fine.
Ok.
There is also the question of where the Vybrid PDB driver would belong
to? In the TRM it is put in the timers but the kernel has no generic timer
framework that I am aware of.
It's been debated a number of times, but no one has ever written one.
Not seen anything recently on this topic.. Lars, you seen anything?
(the blackfin-timer trigger is similar to what you are describing I think).
After some internal reviews we decided to
do a platform driver which provided functions ADC driver could called into.
Does rather feel like there ought to be at least a standard home for these.
Might be worth asking the arm-soc guys...
I have a patchset ready which provides trigger support using PDB however
configuring the PDB properly has proven to be tricky. While it works but
not reliably with multiple channels and it would be a while before I get
that working and post that patchset. So kind of stalled there and just
because of two registers which need to be written with the correct value
:). For what it's worth if someone comes across this, some discussion
here [1] along with patches. (Note however those are a bit old patches
not exactly my new work).
Sorry for digressing from the topic. Anyways so the idea was to provide
sysfs triggers as default for using this continuous sampling. Later the
driver may provide additional triggers. So for now I added the sysfs
trigger as a select option so that a user won't have to recompile the
kernel for using the buffers with continuous sampling.
It's a policy decision so should really be left to the distro builders.
There are lots of possible triggers out there (though sysfs might be
the most likely one!)
Hence don't put the select there in Kconfig. We should shortly have
Daniel's update to the patches for the high resolution timer
which would give another obvious choice for starters.
Alright. Will drop that iio sysfs tigger select from the Kconfig.
I doubt many distros build without the sysfs triggers but with other IIO
stuff.
+ select IIO_TRIGGERED_BUFFER
help
Say yes here to support for Vybrid board analog-to-digital converter.
Since the IP is used for i.MX6SLX, the driver also support i.MX6SLX.
diff --git a/drivers/iio/adc/vf610_adc.c b/drivers/iio/adc/vf610_adc.c
index 23b8fb9..97cb2ed 100644
--- a/drivers/iio/adc/vf610_adc.c
+++ b/drivers/iio/adc/vf610_adc.c
@@ -34,8 +34,11 @@
#include linux/err.h
#include linux/iio/iio.h
+#include linux/iio/buffer.h
#include linux/iio/sysfs.h
-#include linux/iio/driver.h
+#include linux/iio/trigger.h
+#include linux/iio/trigger_consumer.h
+#include linux/iio/triggered_buffer.h
/* This will be the driver name the kernel reports */
#define DRIVER_NAME vf610-adc
@@ -170,6 +173,7 @@ struct vf610_adc {
u32 sample_freq_avail[5];
struct completion completion;
+ u16 buffer[2];
Note the requirements on the buffer provided to
Re: [PATCH v2] iio: adc: vf610: Add IIO buffer support for Vybrid ADC
Hello Jonathan,
On 15-08-08 15:29:47, Jonathan Cameron wrote:
> On 05/08/15 13:10, Sanchayan Maity wrote:
> > This patch adds support for IIO buffer to the Vybrid ADC driver.
> > IIO triggered buffer infrastructure along with iio sysfs trigger
> > is used to leverage continuous sampling support provided by the
> > ADC block.
> Looking good. Just a couple more bits and pieces inline from me.
> One or two of which aren't corrected from Peter's review of v1.
>
> I will want Fugang Dong's ack / review tag on the final version
> before applying it however.
Sure.
> This driver is some distance form my area of expertise!
I doubt :).
>
> Jonathan
> >
> > Signed-off-by: Sanchayan Maity
> > ---
> > drivers/iio/adc/Kconfig | 4 ++
> > drivers/iio/adc/vf610_adc.c | 107
> > +---
> > 2 files changed, 105 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> > index 7c55658..4661241 100644
> > --- a/drivers/iio/adc/Kconfig
> > +++ b/drivers/iio/adc/Kconfig
> > @@ -337,6 +337,10 @@ config TWL6030_GPADC
> > config VF610_ADC
> > tristate "Freescale vf610 ADC driver"
> > depends on OF
> > + select IIO_BUFFER
> > + select IIO_TRIGGER
> > + select IIO_SYSFS_TRIGGER
> Unless I missed something there is no dependency on this particular
> trigger (it just happens to be the one you've been testing with I guess).
> Could be driven from a hardware trigger belonging to another device for
> example.
Yes right in a way. Right now we do not provide or there is no provision
for hardware triggers. On the Vybrid, the Peripheral Delay Block (PDB)
does the job of providing support for software and hardware triggers. PDB
support is not yet there in Linux however.
There is also the question of where the Vybrid PDB driver would belong
to? In the TRM it is put in the timers but the kernel has no generic timer
framework that I am aware of. After some internal reviews we decided to
do a platform driver which provided functions ADC driver could called into.
I have a patchset ready which provides trigger support using PDB however
configuring the PDB properly has proven to be tricky. While it works but
not reliably with multiple channels and it would be a while before I get
that working and post that patchset. So kind of stalled there and just
because of two registers which need to be written with the correct value
:). For what it's worth if someone comes across this, some discussion
here [1] along with patches. (Note however those are a bit old patches
not exactly my new work).
Sorry for digressing from the topic. Anyways so the idea was to provide
sysfs triggers as default for using this continuous sampling. Later the
driver may provide additional triggers. So for now I added the sysfs
trigger as a select option so that a user won't have to recompile the
kernel for using the buffers with continuous sampling.
>
> > + select IIO_TRIGGERED_BUFFER
> > help
> > Say yes here to support for Vybrid board analog-to-digital converter.
> > Since the IP is used for i.MX6SLX, the driver also support i.MX6SLX.
> > diff --git a/drivers/iio/adc/vf610_adc.c b/drivers/iio/adc/vf610_adc.c
> > index 23b8fb9..97cb2ed 100644
> > --- a/drivers/iio/adc/vf610_adc.c
> > +++ b/drivers/iio/adc/vf610_adc.c
> > @@ -34,8 +34,11 @@
> > #include
> >
> > #include
> > +#include
> > #include
> > -#include
> > +#include
> > +#include
> > +#include
> >
> > /* This will be the driver name the kernel reports */
> > #define DRIVER_NAME "vf610-adc"
> > @@ -170,6 +173,7 @@ struct vf610_adc {
> > u32 sample_freq_avail[5];
> >
> > struct completion completion;
> > + u16 buffer[2];
> Note the requirements on the buffer provided to
> iio_push_to_buffers_with_timestamp
> Needs to be u16 buffer[8] to allow for the aligned 64bit (4 word) timestamp.
>
> Peter pointed this out in his follow up email and you said you'd implement
> it.. Guessing this got lost somewhere.
No, I meant to implement what Peter recommended but I guess I did not completely
grasp what he intended. Sorry about that. Will fix this and ask further if in
more doubts.
>
>
> > };
> >
> > static const u32 vf610_hw_avgs[] = { 1, 4, 8, 16, 32 };
> > @@ -505,12 +509,22 @@ static const struct iio_chan_spec_ext_info
> > vf610_ext_info[] = {
> > .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) | \
> > BIT(IIO_CHAN_INFO_SAMP_FREQ), \
> > .ext_info = vf610_ext_info, \
> > + .address = (_idx), \
> > + .scan_index = (_idx), \
> > + .scan_type.sign = 'u', \
> > + .scan_type.realbits = 12, \
> > + .scan_type.storagebits = 16,\
> > }
> >
> > #define VF610_ADC_TEMPERATURE_CHAN(_idx, _chan_type) { \
> > .type = (_chan_type), \
> > .channel = (_idx), \
> >
Re: [PATCH v2] iio: adc: vf610: Add IIO buffer support for Vybrid ADC
Hello Jonathan,
On 15-08-08 15:29:47, Jonathan Cameron wrote:
On 05/08/15 13:10, Sanchayan Maity wrote:
This patch adds support for IIO buffer to the Vybrid ADC driver.
IIO triggered buffer infrastructure along with iio sysfs trigger
is used to leverage continuous sampling support provided by the
ADC block.
Looking good. Just a couple more bits and pieces inline from me.
One or two of which aren't corrected from Peter's review of v1.
I will want Fugang Dong's ack / review tag on the final version
before applying it however.
Sure.
This driver is some distance form my area of expertise!
I doubt :).
Jonathan
Signed-off-by: Sanchayan Maity maitysancha...@gmail.com
---
drivers/iio/adc/Kconfig | 4 ++
drivers/iio/adc/vf610_adc.c | 107
+---
2 files changed, 105 insertions(+), 6 deletions(-)
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 7c55658..4661241 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -337,6 +337,10 @@ config TWL6030_GPADC
config VF610_ADC
tristate Freescale vf610 ADC driver
depends on OF
+ select IIO_BUFFER
+ select IIO_TRIGGER
+ select IIO_SYSFS_TRIGGER
Unless I missed something there is no dependency on this particular
trigger (it just happens to be the one you've been testing with I guess).
Could be driven from a hardware trigger belonging to another device for
example.
Yes right in a way. Right now we do not provide or there is no provision
for hardware triggers. On the Vybrid, the Peripheral Delay Block (PDB)
does the job of providing support for software and hardware triggers. PDB
support is not yet there in Linux however.
There is also the question of where the Vybrid PDB driver would belong
to? In the TRM it is put in the timers but the kernel has no generic timer
framework that I am aware of. After some internal reviews we decided to
do a platform driver which provided functions ADC driver could called into.
I have a patchset ready which provides trigger support using PDB however
configuring the PDB properly has proven to be tricky. While it works but
not reliably with multiple channels and it would be a while before I get
that working and post that patchset. So kind of stalled there and just
because of two registers which need to be written with the correct value
:). For what it's worth if someone comes across this, some discussion
here [1] along with patches. (Note however those are a bit old patches
not exactly my new work).
Sorry for digressing from the topic. Anyways so the idea was to provide
sysfs triggers as default for using this continuous sampling. Later the
driver may provide additional triggers. So for now I added the sysfs
trigger as a select option so that a user won't have to recompile the
kernel for using the buffers with continuous sampling.
+ select IIO_TRIGGERED_BUFFER
help
Say yes here to support for Vybrid board analog-to-digital converter.
Since the IP is used for i.MX6SLX, the driver also support i.MX6SLX.
diff --git a/drivers/iio/adc/vf610_adc.c b/drivers/iio/adc/vf610_adc.c
index 23b8fb9..97cb2ed 100644
--- a/drivers/iio/adc/vf610_adc.c
+++ b/drivers/iio/adc/vf610_adc.c
@@ -34,8 +34,11 @@
#include linux/err.h
#include linux/iio/iio.h
+#include linux/iio/buffer.h
#include linux/iio/sysfs.h
-#include linux/iio/driver.h
+#include linux/iio/trigger.h
+#include linux/iio/trigger_consumer.h
+#include linux/iio/triggered_buffer.h
/* This will be the driver name the kernel reports */
#define DRIVER_NAME vf610-adc
@@ -170,6 +173,7 @@ struct vf610_adc {
u32 sample_freq_avail[5];
struct completion completion;
+ u16 buffer[2];
Note the requirements on the buffer provided to
iio_push_to_buffers_with_timestamp
Needs to be u16 buffer[8] to allow for the aligned 64bit (4 word) timestamp.
Peter pointed this out in his follow up email and you said you'd implement
it.. Guessing this got lost somewhere.
No, I meant to implement what Peter recommended but I guess I did not completely
grasp what he intended. Sorry about that. Will fix this and ask further if in
more doubts.
};
static const u32 vf610_hw_avgs[] = { 1, 4, 8, 16, 32 };
@@ -505,12 +509,22 @@ static const struct iio_chan_spec_ext_info
vf610_ext_info[] = {
.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) | \
BIT(IIO_CHAN_INFO_SAMP_FREQ), \
.ext_info = vf610_ext_info, \
+ .address = (_idx), \
+ .scan_index = (_idx), \
+ .scan_type.sign = 'u', \
+ .scan_type.realbits = 12, \
+ .scan_type.storagebits = 16,\
}
#define VF610_ADC_TEMPERATURE_CHAN(_idx, _chan_type) { \
.type = (_chan_type), \
.channel = (_idx), \
Re: [PATCH] iio: adc: vf610: Add IIO buffer support for Vybrid ADC
Hello,
On 15-08-04 03:18:46, Duan Andy wrote:
> From: Sanchayan Maity Sent: Monday, August 03,
> 2015 11:10 PM
> > To: ji...@kernel.org; linux-...@vger.kernel.org
> > Cc: ste...@agner.ch; Duan Fugang-B38611; hof...@osadl.org;
> > sanjeev_sha...@mentor.com; Estevam Fabio-R49496; knaac...@gmx.de;
> > l...@metafoo.de; pme...@pmeerw.net; antoine.ten...@free-electrons.com;
> > linux-kernel@vger.kernel.org; linux-arm-ker...@lists.infradead.org;
> > Sanchayan Maity
> > Subject: [PATCH] iio: adc: vf610: Add IIO buffer support for Vybrid ADC
> >
> > This patch adds support for IIO buffer to the Vybrid ADC driver.
> > IIO triggered buffer infrastructure along with iio sysfs trigger is used
> > to leverage continuous sampling support provided by the ADC block.
> >
> > Signed-off-by: Sanchayan Maity
> > ---
> > drivers/iio/adc/Kconfig | 4 ++
> > drivers/iio/adc/vf610_adc.c | 122
> > +---
> > 2 files changed, 120 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig index
> > 7c55658..4661241 100644
> > --- a/drivers/iio/adc/Kconfig
> > +++ b/drivers/iio/adc/Kconfig
> > @@ -337,6 +337,10 @@ config TWL6030_GPADC config VF610_ADC
> > tristate "Freescale vf610 ADC driver"
> > depends on OF
> > + select IIO_BUFFER
> > + select IIO_TRIGGER
> > + select IIO_SYSFS_TRIGGER
> > + select IIO_TRIGGERED_BUFFER
> > help
> > Say yes here to support for Vybrid board analog-to-digital
> > converter.
> > Since the IP is used for i.MX6SLX, the driver also support
> > i.MX6SLX.
> > diff --git a/drivers/iio/adc/vf610_adc.c b/drivers/iio/adc/vf610_adc.c
> > index 23b8fb9..af72b9a 100644
> > --- a/drivers/iio/adc/vf610_adc.c
> > +++ b/drivers/iio/adc/vf610_adc.c
> > @@ -34,8 +34,11 @@
> > #include
> >
> > #include
> > +#include
> > #include
> > -#include
> > +#include
> > +#include #include
> > +
> >
> > /* This will be the driver name the kernel reports */ #define
> > DRIVER_NAME "vf610-adc"
> > @@ -170,6 +173,7 @@ struct vf610_adc {
> > u32 sample_freq_avail[5];
> >
> > struct completion completion;
> > + u16 *buffer;
> > };
> >
> > static const u32 vf610_hw_avgs[] = { 1, 4, 8, 16, 32 }; @@ -505,12
> > +509,22 @@ static const struct iio_chan_spec_ext_info vf610_ext_info[] =
> > {
> > .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) | \
> > BIT(IIO_CHAN_INFO_SAMP_FREQ), \
> > .ext_info = vf610_ext_info, \
> > + .address = (_idx), \
> > + .scan_index = (_idx), \
> > + .scan_type.sign = 'u', \
> > + .scan_type.realbits = 12, \
> > + .scan_type.storagebits = 16,\
> > }
> >
> > #define VF610_ADC_TEMPERATURE_CHAN(_idx, _chan_type) { \
> > .type = (_chan_type), \
> > .channel = (_idx), \
> > .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED), \
> > + .address = (_idx), \
> > + .scan_index = (_idx), \
> > + .scan_type.sign = 'u', \
> > + .scan_type.realbits = 12, \
> > + .scan_type.storagebits = 16,\
> > }
> >
> > static const struct iio_chan_spec vf610_adc_iio_channels[] = { @@ -531,6
> > +545,7 @@ static const struct iio_chan_spec vf610_adc_iio_channels[] = {
> > VF610_ADC_CHAN(14, IIO_VOLTAGE),
> > VF610_ADC_CHAN(15, IIO_VOLTAGE),
> > VF610_ADC_TEMPERATURE_CHAN(26, IIO_TEMP),
> > + IIO_CHAN_SOFT_TIMESTAMP(32),
> > /* sentinel */
> > };
> >
> > @@ -559,13 +574,21 @@ static int vf610_adc_read_data(struct vf610_adc
> > *info)
> >
> > static irqreturn_t vf610_adc_isr(int irq, void *dev_id) {
> > - struct vf610_adc *info = (struct vf610_adc *)dev_id;
> > + struct iio_dev *indio_dev = (struct iio_dev *)dev_id;
> > + struct vf610_adc *info = iio_priv(indio_dev);
> > int coco;
> >
> > coco = readl(info->regs + VF610_REG_ADC_HS);
> > if (coco & VF610_ADC_HS_COCO0) {
> > info->value = vf610_adc_read_data(info);
> > - complete(>completion);
> > + if (iio_buffer_enabled(indio_dev)) {
> > + info->buffer[0] = info->value;
> > + writel(0, info->regs + VF610_REG_ADC_HS);
> The register is read only. After ADC_Rn is read, the coco bit is cleared.
Right. This crept in from a different implementation which I was trying with
PDB and perhaps a wrong interpretation of the TRM.
Will fix.
Regards,
Sanchayan.
(snip)
--
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: [PATCH v2 3/4] touchscreen: colibri-vf50-ts: Add touchscreen support for Colibri VF50
Hello Dmitry,
On 15-08-03 14:04:09, Dmitry Torokhov wrote:
> Hi Sanchayan,
>
> On Mon, Aug 03, 2015 at 08:55:44PM +0530, maitysancha...@gmail.com wrote:
> > Hello Dmitry,
> >
> > On 15-07-21 10:20:44, Dmitry Torokhov wrote:
> > > Hi Stefan,
> > >
> > > On Tue, Jul 21, 2015 at 04:43:36PM +0200, Stefan Agner wrote:
> > > > Hi Dmitry,
> > > >
> > > > As the original author of the driver I have some remarks to your review
> > > >
> > > > On 2015-07-18 01:42, Dmitry Torokhov wrote:
> > > > >> +/*
> > > > >> + * If touch pressure is too low, stop measuring and
> > > > >> reenable
> > > > >> + * touch detection
> > > > >> + */
> > > > >> +if (val_p < min_pressure || val_p > 2000)
> > > > >> +break;
> > > >
> > > > This is where the modules touch pressure is used to stop the measurement
> > > > process and switch back to interrupt mode. See my remarks at the end.
> > > >
> > > > >> +
> > > > >> +/*
> > > > >> + * The pressure may not be enough for the first x and
> > > > >> the
> > > > >> + * second y measurement, but, the pressure is ok when
> > > > >> the
> > > > >> + * driver is doing the third and fourth measurement. To
> > > > >> + * take care of this, we drop the first measurement
> > > > >> always.
> > > > >> + */
> > > > >> +if (discard_val_on_start) {
> > > > >> +discard_val_on_start = false;
> > > > >> +} else {
> > > > >> +/*
> > > > >> + * Report touch position and sleep for
> > > > >> + * next measurement
> > > > >> + */
> > > > >> +input_report_abs(vf50_ts->ts_input,
> > > > >> +ABS_X, VF_ADC_MAX - val_x);
> > > > >> +input_report_abs(vf50_ts->ts_input,
> > > > >> +ABS_Y, VF_ADC_MAX - val_y);
> > > > >> +input_report_abs(vf50_ts->ts_input,
> > > > >> +ABS_PRESSURE, val_p);
> > > > >> +input_report_key(vf50_ts->ts_input, BTN_TOUCH,
> > > > >> 1);
> > > > >> +input_sync(vf50_ts->ts_input);
> > > > >> +}
> > > > >> +
> > > > >> +msleep(10);
> > > > >> +}
> > > > >> +
> > > > >> +/* Report no more touch, reenable touch detection */
> > > > >> +input_report_abs(vf50_ts->ts_input, ABS_PRESSURE, 0);
> > > > >> +input_report_key(vf50_ts->ts_input, BTN_TOUCH, 0);
> > > > >> +input_sync(vf50_ts->ts_input);
> > > > >> +
> > > > >> +vf50_ts_enable_touch_detection(vf50_ts);
> > > > >> +
> > > > >> +/* Wait for the pull-up to be stable on high */
> > > > >> +msleep(10);
> > > > >> +
> > > > >> +/* Reenable IRQ to detect touch */
> > > > >> +enable_irq(vf50_ts->pen_irq);
> > > > >> +
> > > > >> +dev_dbg(dev, "Reenabled touch detection interrupt\n");
> > > > >> +}
> > > > >> +
> > > > >> +static irqreturn_t vf50_ts_touched(int irq, void *dev_id)
> > > > >> +{
> > > > >> +struct vf50_touch_device *vf50_ts = (struct vf50_touch_device
> > > > >> *)dev_id;
> > > > >> +struct device *dev = _ts->pdev->dev;
> > > > >> +
> > > > >> +dev_dbg(dev, "Touch detected, start worker thread\n");
> > > > >> +
> > > > >> +disable_irq_nosync(irq);
> > > > >> +
> > > > >> +/* Disable the touch detection plates */
> > > > >> +gpiod_set_value(vf50_ts->gpio_ym, 0);
> > > > >> +
> > > > >> +/* Let the platform mux to default state in order to mux as ADC
> > > > >> */
> > > > >> +pinctrl_pm_select_default_state(dev);
> > > > >> +
> > > > >> +queue_work(vf50_ts->ts_workqueue, _ts->ts_work);
> > > > >
> > > > > If you convert this to a threaded interrupt you won't need to
> > > > > disable/reenable interrupt or queue work. You should also be able to
> > > > > use
> > > > > gpiod_set_value_cansleep() extending the range of ways the controller
> > > > > could be connected to systems.
> > > > >
> > > >
> > > > I'm not sure if a threaded interrupt is the right thing here. While the
> > > > pen is on the touchscreen (which can be for several seconds)
> > > > measurements have to be made in a continuous loop. Is it ok for a
> > > > threaded interrupt to run that long?
> > >
> > > Yes, why not? Threaded interrupt is simply a kernel thread that is woken
> > > when hard interrupt handler tells it to wake up. Very similar to
> > > interrupt + work queue, except that the kernel manages interactions
> > > properly for you. There are several drivers in kernel that do that, for
> > > example auo-pixcir-ts.c or tsc2007.c
> > >
> > > >
> > > > I'm also not sure if it is really safe to _not_ disable the pen down
> > > > GPIO interrupt. If we get a interrupt while measuring, we should ignore
> > > > that interrupt.
> > >
> > > The interrupt management core
Re: [PATCH v2 3/4] touchscreen: colibri-vf50-ts: Add touchscreen support for Colibri VF50
Hello Dmitry,
On 15-08-03 14:04:09, Dmitry Torokhov wrote:
Hi Sanchayan,
On Mon, Aug 03, 2015 at 08:55:44PM +0530, maitysancha...@gmail.com wrote:
Hello Dmitry,
On 15-07-21 10:20:44, Dmitry Torokhov wrote:
Hi Stefan,
On Tue, Jul 21, 2015 at 04:43:36PM +0200, Stefan Agner wrote:
Hi Dmitry,
As the original author of the driver I have some remarks to your review
On 2015-07-18 01:42, Dmitry Torokhov wrote:
+/*
+ * If touch pressure is too low, stop measuring and
reenable
+ * touch detection
+ */
+if (val_p min_pressure || val_p 2000)
+break;
This is where the modules touch pressure is used to stop the measurement
process and switch back to interrupt mode. See my remarks at the end.
+
+/*
+ * The pressure may not be enough for the first x and
the
+ * second y measurement, but, the pressure is ok when
the
+ * driver is doing the third and fourth measurement. To
+ * take care of this, we drop the first measurement
always.
+ */
+if (discard_val_on_start) {
+discard_val_on_start = false;
+} else {
+/*
+ * Report touch position and sleep for
+ * next measurement
+ */
+input_report_abs(vf50_ts-ts_input,
+ABS_X, VF_ADC_MAX - val_x);
+input_report_abs(vf50_ts-ts_input,
+ABS_Y, VF_ADC_MAX - val_y);
+input_report_abs(vf50_ts-ts_input,
+ABS_PRESSURE, val_p);
+input_report_key(vf50_ts-ts_input, BTN_TOUCH,
1);
+input_sync(vf50_ts-ts_input);
+}
+
+msleep(10);
+}
+
+/* Report no more touch, reenable touch detection */
+input_report_abs(vf50_ts-ts_input, ABS_PRESSURE, 0);
+input_report_key(vf50_ts-ts_input, BTN_TOUCH, 0);
+input_sync(vf50_ts-ts_input);
+
+vf50_ts_enable_touch_detection(vf50_ts);
+
+/* Wait for the pull-up to be stable on high */
+msleep(10);
+
+/* Reenable IRQ to detect touch */
+enable_irq(vf50_ts-pen_irq);
+
+dev_dbg(dev, Reenabled touch detection interrupt\n);
+}
+
+static irqreturn_t vf50_ts_touched(int irq, void *dev_id)
+{
+struct vf50_touch_device *vf50_ts = (struct vf50_touch_device
*)dev_id;
+struct device *dev = vf50_ts-pdev-dev;
+
+dev_dbg(dev, Touch detected, start worker thread\n);
+
+disable_irq_nosync(irq);
+
+/* Disable the touch detection plates */
+gpiod_set_value(vf50_ts-gpio_ym, 0);
+
+/* Let the platform mux to default state in order to mux as ADC
*/
+pinctrl_pm_select_default_state(dev);
+
+queue_work(vf50_ts-ts_workqueue, vf50_ts-ts_work);
If you convert this to a threaded interrupt you won't need to
disable/reenable interrupt or queue work. You should also be able to
use
gpiod_set_value_cansleep() extending the range of ways the controller
could be connected to systems.
I'm not sure if a threaded interrupt is the right thing here. While the
pen is on the touchscreen (which can be for several seconds)
measurements have to be made in a continuous loop. Is it ok for a
threaded interrupt to run that long?
Yes, why not? Threaded interrupt is simply a kernel thread that is woken
when hard interrupt handler tells it to wake up. Very similar to
interrupt + work queue, except that the kernel manages interactions
properly for you. There are several drivers in kernel that do that, for
example auo-pixcir-ts.c or tsc2007.c
I'm also not sure if it is really safe to _not_ disable the pen down
GPIO interrupt. If we get a interrupt while measuring, we should ignore
that interrupt.
The interrupt management core (you'll have to annotate it as
IRQF_ONESHOT) will make sure it stays masked properly until the threaded
handler completes so you do not need to disable it explicitly.
After working some more on threaded irq implementation, if IRQ_ONESHOT
flag is used while requesting threaded irq, on entering the IRQ handler
the vf610_gpio_irq_mask is not called and vf610_gpio_irq_unmask is not
called on exiting the thread function, not something we expected.
In contrast, using explicit disable_irq_nosync and enable_irq in the IRQ
handler and thread respectively results in the
Re: [PATCH] iio: adc: vf610: Add IIO buffer support for Vybrid ADC
Hello,
On 15-08-04 03:18:46, Duan Andy wrote:
From: Sanchayan Maity maitysancha...@gmail.com Sent: Monday, August 03,
2015 11:10 PM
To: ji...@kernel.org; linux-...@vger.kernel.org
Cc: ste...@agner.ch; Duan Fugang-B38611; hof...@osadl.org;
sanjeev_sha...@mentor.com; Estevam Fabio-R49496; knaac...@gmx.de;
l...@metafoo.de; pme...@pmeerw.net; antoine.ten...@free-electrons.com;
linux-kernel@vger.kernel.org; linux-arm-ker...@lists.infradead.org;
Sanchayan Maity
Subject: [PATCH] iio: adc: vf610: Add IIO buffer support for Vybrid ADC
This patch adds support for IIO buffer to the Vybrid ADC driver.
IIO triggered buffer infrastructure along with iio sysfs trigger is used
to leverage continuous sampling support provided by the ADC block.
Signed-off-by: Sanchayan Maity maitysancha...@gmail.com
---
drivers/iio/adc/Kconfig | 4 ++
drivers/iio/adc/vf610_adc.c | 122
+---
2 files changed, 120 insertions(+), 6 deletions(-)
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig index
7c55658..4661241 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -337,6 +337,10 @@ config TWL6030_GPADC config VF610_ADC
tristate Freescale vf610 ADC driver
depends on OF
+ select IIO_BUFFER
+ select IIO_TRIGGER
+ select IIO_SYSFS_TRIGGER
+ select IIO_TRIGGERED_BUFFER
help
Say yes here to support for Vybrid board analog-to-digital
converter.
Since the IP is used for i.MX6SLX, the driver also support
i.MX6SLX.
diff --git a/drivers/iio/adc/vf610_adc.c b/drivers/iio/adc/vf610_adc.c
index 23b8fb9..af72b9a 100644
--- a/drivers/iio/adc/vf610_adc.c
+++ b/drivers/iio/adc/vf610_adc.c
@@ -34,8 +34,11 @@
#include linux/err.h
#include linux/iio/iio.h
+#include linux/iio/buffer.h
#include linux/iio/sysfs.h
-#include linux/iio/driver.h
+#include linux/iio/trigger.h
+#include linux/iio/trigger_consumer.h #include
+linux/iio/triggered_buffer.h
/* This will be the driver name the kernel reports */ #define
DRIVER_NAME vf610-adc
@@ -170,6 +173,7 @@ struct vf610_adc {
u32 sample_freq_avail[5];
struct completion completion;
+ u16 *buffer;
};
static const u32 vf610_hw_avgs[] = { 1, 4, 8, 16, 32 }; @@ -505,12
+509,22 @@ static const struct iio_chan_spec_ext_info vf610_ext_info[] =
{
.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) | \
BIT(IIO_CHAN_INFO_SAMP_FREQ), \
.ext_info = vf610_ext_info, \
+ .address = (_idx), \
+ .scan_index = (_idx), \
+ .scan_type.sign = 'u', \
+ .scan_type.realbits = 12, \
+ .scan_type.storagebits = 16,\
}
#define VF610_ADC_TEMPERATURE_CHAN(_idx, _chan_type) { \
.type = (_chan_type), \
.channel = (_idx), \
.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED), \
+ .address = (_idx), \
+ .scan_index = (_idx), \
+ .scan_type.sign = 'u', \
+ .scan_type.realbits = 12, \
+ .scan_type.storagebits = 16,\
}
static const struct iio_chan_spec vf610_adc_iio_channels[] = { @@ -531,6
+545,7 @@ static const struct iio_chan_spec vf610_adc_iio_channels[] = {
VF610_ADC_CHAN(14, IIO_VOLTAGE),
VF610_ADC_CHAN(15, IIO_VOLTAGE),
VF610_ADC_TEMPERATURE_CHAN(26, IIO_TEMP),
+ IIO_CHAN_SOFT_TIMESTAMP(32),
/* sentinel */
};
@@ -559,13 +574,21 @@ static int vf610_adc_read_data(struct vf610_adc
*info)
static irqreturn_t vf610_adc_isr(int irq, void *dev_id) {
- struct vf610_adc *info = (struct vf610_adc *)dev_id;
+ struct iio_dev *indio_dev = (struct iio_dev *)dev_id;
+ struct vf610_adc *info = iio_priv(indio_dev);
int coco;
coco = readl(info-regs + VF610_REG_ADC_HS);
if (coco VF610_ADC_HS_COCO0) {
info-value = vf610_adc_read_data(info);
- complete(info-completion);
+ if (iio_buffer_enabled(indio_dev)) {
+ info-buffer[0] = info-value;
+ writel(0, info-regs + VF610_REG_ADC_HS);
The register is read only. After ADC_Rn is read, the coco bit is cleared.
Right. This crept in from a different implementation which I was trying with
PDB and perhaps a wrong interpretation of the TRM.
Will fix.
Regards,
Sanchayan.
(snip)
--
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: [PATCH v2 3/4] touchscreen: colibri-vf50-ts: Add touchscreen support for Colibri VF50
Hello Dmitry,
On 15-07-21 10:20:44, Dmitry Torokhov wrote:
> Hi Stefan,
>
> On Tue, Jul 21, 2015 at 04:43:36PM +0200, Stefan Agner wrote:
> > Hi Dmitry,
> >
> > As the original author of the driver I have some remarks to your review
> >
> > On 2015-07-18 01:42, Dmitry Torokhov wrote:
> > >> +/*
> > >> + * If touch pressure is too low, stop measuring and
> > >> reenable
> > >> + * touch detection
> > >> + */
> > >> +if (val_p < min_pressure || val_p > 2000)
> > >> +break;
> >
> > This is where the modules touch pressure is used to stop the measurement
> > process and switch back to interrupt mode. See my remarks at the end.
> >
> > >> +
> > >> +/*
> > >> + * The pressure may not be enough for the first x and
> > >> the
> > >> + * second y measurement, but, the pressure is ok when
> > >> the
> > >> + * driver is doing the third and fourth measurement. To
> > >> + * take care of this, we drop the first measurement
> > >> always.
> > >> + */
> > >> +if (discard_val_on_start) {
> > >> +discard_val_on_start = false;
> > >> +} else {
> > >> +/*
> > >> + * Report touch position and sleep for
> > >> + * next measurement
> > >> + */
> > >> +input_report_abs(vf50_ts->ts_input,
> > >> +ABS_X, VF_ADC_MAX - val_x);
> > >> +input_report_abs(vf50_ts->ts_input,
> > >> +ABS_Y, VF_ADC_MAX - val_y);
> > >> +input_report_abs(vf50_ts->ts_input,
> > >> +ABS_PRESSURE, val_p);
> > >> +input_report_key(vf50_ts->ts_input, BTN_TOUCH,
> > >> 1);
> > >> +input_sync(vf50_ts->ts_input);
> > >> +}
> > >> +
> > >> +msleep(10);
> > >> +}
> > >> +
> > >> +/* Report no more touch, reenable touch detection */
> > >> +input_report_abs(vf50_ts->ts_input, ABS_PRESSURE, 0);
> > >> +input_report_key(vf50_ts->ts_input, BTN_TOUCH, 0);
> > >> +input_sync(vf50_ts->ts_input);
> > >> +
> > >> +vf50_ts_enable_touch_detection(vf50_ts);
> > >> +
> > >> +/* Wait for the pull-up to be stable on high */
> > >> +msleep(10);
> > >> +
> > >> +/* Reenable IRQ to detect touch */
> > >> +enable_irq(vf50_ts->pen_irq);
> > >> +
> > >> +dev_dbg(dev, "Reenabled touch detection interrupt\n");
> > >> +}
> > >> +
> > >> +static irqreturn_t vf50_ts_touched(int irq, void *dev_id)
> > >> +{
> > >> +struct vf50_touch_device *vf50_ts = (struct vf50_touch_device
> > >> *)dev_id;
> > >> +struct device *dev = _ts->pdev->dev;
> > >> +
> > >> +dev_dbg(dev, "Touch detected, start worker thread\n");
> > >> +
> > >> +disable_irq_nosync(irq);
> > >> +
> > >> +/* Disable the touch detection plates */
> > >> +gpiod_set_value(vf50_ts->gpio_ym, 0);
> > >> +
> > >> +/* Let the platform mux to default state in order to mux as ADC
> > >> */
> > >> +pinctrl_pm_select_default_state(dev);
> > >> +
> > >> +queue_work(vf50_ts->ts_workqueue, _ts->ts_work);
> > >
> > > If you convert this to a threaded interrupt you won't need to
> > > disable/reenable interrupt or queue work. You should also be able to use
> > > gpiod_set_value_cansleep() extending the range of ways the controller
> > > could be connected to systems.
> > >
> >
> > I'm not sure if a threaded interrupt is the right thing here. While the
> > pen is on the touchscreen (which can be for several seconds)
> > measurements have to be made in a continuous loop. Is it ok for a
> > threaded interrupt to run that long?
>
> Yes, why not? Threaded interrupt is simply a kernel thread that is woken
> when hard interrupt handler tells it to wake up. Very similar to
> interrupt + work queue, except that the kernel manages interactions
> properly for you. There are several drivers in kernel that do that, for
> example auo-pixcir-ts.c or tsc2007.c
>
> >
> > I'm also not sure if it is really safe to _not_ disable the pen down
> > GPIO interrupt. If we get a interrupt while measuring, we should ignore
> > that interrupt.
>
> The interrupt management core (you'll have to annotate it as
> IRQF_ONESHOT) will make sure it stays masked properly until the threaded
> handler completes so you do not need to disable it explicitly.
After working some more on threaded irq implementation, if IRQ_ONESHOT
flag is used while requesting threaded irq, on entering the IRQ handler
the vf610_gpio_irq_mask is not called and vf610_gpio_irq_unmask is not
called on exiting the thread
Re: [PATCH v2 3/4] touchscreen: colibri-vf50-ts: Add touchscreen support for Colibri VF50
Hello Dmitry,
On 15-07-21 10:20:44, Dmitry Torokhov wrote:
Hi Stefan,
On Tue, Jul 21, 2015 at 04:43:36PM +0200, Stefan Agner wrote:
Hi Dmitry,
As the original author of the driver I have some remarks to your review
On 2015-07-18 01:42, Dmitry Torokhov wrote:
+/*
+ * If touch pressure is too low, stop measuring and
reenable
+ * touch detection
+ */
+if (val_p min_pressure || val_p 2000)
+break;
This is where the modules touch pressure is used to stop the measurement
process and switch back to interrupt mode. See my remarks at the end.
+
+/*
+ * The pressure may not be enough for the first x and
the
+ * second y measurement, but, the pressure is ok when
the
+ * driver is doing the third and fourth measurement. To
+ * take care of this, we drop the first measurement
always.
+ */
+if (discard_val_on_start) {
+discard_val_on_start = false;
+} else {
+/*
+ * Report touch position and sleep for
+ * next measurement
+ */
+input_report_abs(vf50_ts-ts_input,
+ABS_X, VF_ADC_MAX - val_x);
+input_report_abs(vf50_ts-ts_input,
+ABS_Y, VF_ADC_MAX - val_y);
+input_report_abs(vf50_ts-ts_input,
+ABS_PRESSURE, val_p);
+input_report_key(vf50_ts-ts_input, BTN_TOUCH,
1);
+input_sync(vf50_ts-ts_input);
+}
+
+msleep(10);
+}
+
+/* Report no more touch, reenable touch detection */
+input_report_abs(vf50_ts-ts_input, ABS_PRESSURE, 0);
+input_report_key(vf50_ts-ts_input, BTN_TOUCH, 0);
+input_sync(vf50_ts-ts_input);
+
+vf50_ts_enable_touch_detection(vf50_ts);
+
+/* Wait for the pull-up to be stable on high */
+msleep(10);
+
+/* Reenable IRQ to detect touch */
+enable_irq(vf50_ts-pen_irq);
+
+dev_dbg(dev, Reenabled touch detection interrupt\n);
+}
+
+static irqreturn_t vf50_ts_touched(int irq, void *dev_id)
+{
+struct vf50_touch_device *vf50_ts = (struct vf50_touch_device
*)dev_id;
+struct device *dev = vf50_ts-pdev-dev;
+
+dev_dbg(dev, Touch detected, start worker thread\n);
+
+disable_irq_nosync(irq);
+
+/* Disable the touch detection plates */
+gpiod_set_value(vf50_ts-gpio_ym, 0);
+
+/* Let the platform mux to default state in order to mux as ADC
*/
+pinctrl_pm_select_default_state(dev);
+
+queue_work(vf50_ts-ts_workqueue, vf50_ts-ts_work);
If you convert this to a threaded interrupt you won't need to
disable/reenable interrupt or queue work. You should also be able to use
gpiod_set_value_cansleep() extending the range of ways the controller
could be connected to systems.
I'm not sure if a threaded interrupt is the right thing here. While the
pen is on the touchscreen (which can be for several seconds)
measurements have to be made in a continuous loop. Is it ok for a
threaded interrupt to run that long?
Yes, why not? Threaded interrupt is simply a kernel thread that is woken
when hard interrupt handler tells it to wake up. Very similar to
interrupt + work queue, except that the kernel manages interactions
properly for you. There are several drivers in kernel that do that, for
example auo-pixcir-ts.c or tsc2007.c
I'm also not sure if it is really safe to _not_ disable the pen down
GPIO interrupt. If we get a interrupt while measuring, we should ignore
that interrupt.
The interrupt management core (you'll have to annotate it as
IRQF_ONESHOT) will make sure it stays masked properly until the threaded
handler completes so you do not need to disable it explicitly.
After working some more on threaded irq implementation, if IRQ_ONESHOT
flag is used while requesting threaded irq, on entering the IRQ handler
the vf610_gpio_irq_mask is not called and vf610_gpio_irq_unmask is not
called on exiting the thread function, not something we expected.
In contrast, using explicit disable_irq_nosync and enable_irq in the IRQ
handler and thread respectively results in the respective mask and unmask
function being called.
The vf610_gpio_irq_*mask functions are in the gpio driver for Vybrid in
drivers/gpio/gpio-vf610.c.
Can you point me in the right direction?
Thanks Regards,
Re: [PATCH v2 3/4] touchscreen: colibri-vf50-ts: Add touchscreen support for Colibri VF50
On 15-07-21 10:20:44, Dmitry Torokhov wrote:
> Hi Stefan,
>
> On Tue, Jul 21, 2015 at 04:43:36PM +0200, Stefan Agner wrote:
> > Hi Dmitry,
> >
> > As the original author of the driver I have some remarks to your review
> >
> > On 2015-07-18 01:42, Dmitry Torokhov wrote:
> > >> +/*
> > >> + * If touch pressure is too low, stop measuring and
> > >> reenable
> > >> + * touch detection
> > >> + */
> > >> +if (val_p < min_pressure || val_p > 2000)
> > >> +break;
> >
> > This is where the modules touch pressure is used to stop the measurement
> > process and switch back to interrupt mode. See my remarks at the end.
> >
> > >> +
> > >> +/*
> > >> + * The pressure may not be enough for the first x and
> > >> the
> > >> + * second y measurement, but, the pressure is ok when
> > >> the
> > >> + * driver is doing the third and fourth measurement. To
> > >> + * take care of this, we drop the first measurement
> > >> always.
> > >> + */
> > >> +if (discard_val_on_start) {
> > >> +discard_val_on_start = false;
> > >> +} else {
> > >> +/*
> > >> + * Report touch position and sleep for
> > >> + * next measurement
> > >> + */
> > >> +input_report_abs(vf50_ts->ts_input,
> > >> +ABS_X, VF_ADC_MAX - val_x);
> > >> +input_report_abs(vf50_ts->ts_input,
> > >> +ABS_Y, VF_ADC_MAX - val_y);
> > >> +input_report_abs(vf50_ts->ts_input,
> > >> +ABS_PRESSURE, val_p);
> > >> +input_report_key(vf50_ts->ts_input, BTN_TOUCH,
> > >> 1);
> > >> +input_sync(vf50_ts->ts_input);
> > >> +}
> > >> +
> > >> +msleep(10);
> > >> +}
> > >> +
> > >> +/* Report no more touch, reenable touch detection */
> > >> +input_report_abs(vf50_ts->ts_input, ABS_PRESSURE, 0);
> > >> +input_report_key(vf50_ts->ts_input, BTN_TOUCH, 0);
> > >> +input_sync(vf50_ts->ts_input);
> > >> +
> > >> +vf50_ts_enable_touch_detection(vf50_ts);
> > >> +
> > >> +/* Wait for the pull-up to be stable on high */
> > >> +msleep(10);
> > >> +
> > >> +/* Reenable IRQ to detect touch */
> > >> +enable_irq(vf50_ts->pen_irq);
> > >> +
> > >> +dev_dbg(dev, "Reenabled touch detection interrupt\n");
> > >> +}
> > >> +
> > >> +static irqreturn_t vf50_ts_touched(int irq, void *dev_id)
> > >> +{
> > >> +struct vf50_touch_device *vf50_ts = (struct vf50_touch_device
> > >> *)dev_id;
> > >> +struct device *dev = _ts->pdev->dev;
> > >> +
> > >> +dev_dbg(dev, "Touch detected, start worker thread\n");
> > >> +
> > >> +disable_irq_nosync(irq);
> > >> +
> > >> +/* Disable the touch detection plates */
> > >> +gpiod_set_value(vf50_ts->gpio_ym, 0);
> > >> +
> > >> +/* Let the platform mux to default state in order to mux as ADC
> > >> */
> > >> +pinctrl_pm_select_default_state(dev);
> > >> +
> > >> +queue_work(vf50_ts->ts_workqueue, _ts->ts_work);
> > >
> > > If you convert this to a threaded interrupt you won't need to
> > > disable/reenable interrupt or queue work. You should also be able to use
> > > gpiod_set_value_cansleep() extending the range of ways the controller
> > > could be connected to systems.
> > >
> >
> > I'm not sure if a threaded interrupt is the right thing here. While the
> > pen is on the touchscreen (which can be for several seconds)
> > measurements have to be made in a continuous loop. Is it ok for a
> > threaded interrupt to run that long?
>
> Yes, why not? Threaded interrupt is simply a kernel thread that is woken
> when hard interrupt handler tells it to wake up. Very similar to
> interrupt + work queue, except that the kernel manages interactions
> properly for you. There are several drivers in kernel that do that, for
> example auo-pixcir-ts.c or tsc2007.c
>
> >
> > I'm also not sure if it is really safe to _not_ disable the pen down
> > GPIO interrupt. If we get a interrupt while measuring, we should ignore
> > that interrupt.
>
> The interrupt management core (you'll have to annotate it as
> IRQF_ONESHOT) will make sure it stays masked properly until the threaded
> handler completes so you do not need to disable it explicitly.
(snip)
I tried the IRQ threaded implementation. From your reply, I can see my first
implementation was wrong in the sense that I did not use the IRQF_ONESHOT flag.
The touch response time was not good in this case, however thats to be expected
in this case
Re: [PATCH v2 3/4] touchscreen: colibri-vf50-ts: Add touchscreen support for Colibri VF50
On 15-07-21 10:20:44, Dmitry Torokhov wrote:
Hi Stefan,
On Tue, Jul 21, 2015 at 04:43:36PM +0200, Stefan Agner wrote:
Hi Dmitry,
As the original author of the driver I have some remarks to your review
On 2015-07-18 01:42, Dmitry Torokhov wrote:
+/*
+ * If touch pressure is too low, stop measuring and
reenable
+ * touch detection
+ */
+if (val_p min_pressure || val_p 2000)
+break;
This is where the modules touch pressure is used to stop the measurement
process and switch back to interrupt mode. See my remarks at the end.
+
+/*
+ * The pressure may not be enough for the first x and
the
+ * second y measurement, but, the pressure is ok when
the
+ * driver is doing the third and fourth measurement. To
+ * take care of this, we drop the first measurement
always.
+ */
+if (discard_val_on_start) {
+discard_val_on_start = false;
+} else {
+/*
+ * Report touch position and sleep for
+ * next measurement
+ */
+input_report_abs(vf50_ts-ts_input,
+ABS_X, VF_ADC_MAX - val_x);
+input_report_abs(vf50_ts-ts_input,
+ABS_Y, VF_ADC_MAX - val_y);
+input_report_abs(vf50_ts-ts_input,
+ABS_PRESSURE, val_p);
+input_report_key(vf50_ts-ts_input, BTN_TOUCH,
1);
+input_sync(vf50_ts-ts_input);
+}
+
+msleep(10);
+}
+
+/* Report no more touch, reenable touch detection */
+input_report_abs(vf50_ts-ts_input, ABS_PRESSURE, 0);
+input_report_key(vf50_ts-ts_input, BTN_TOUCH, 0);
+input_sync(vf50_ts-ts_input);
+
+vf50_ts_enable_touch_detection(vf50_ts);
+
+/* Wait for the pull-up to be stable on high */
+msleep(10);
+
+/* Reenable IRQ to detect touch */
+enable_irq(vf50_ts-pen_irq);
+
+dev_dbg(dev, Reenabled touch detection interrupt\n);
+}
+
+static irqreturn_t vf50_ts_touched(int irq, void *dev_id)
+{
+struct vf50_touch_device *vf50_ts = (struct vf50_touch_device
*)dev_id;
+struct device *dev = vf50_ts-pdev-dev;
+
+dev_dbg(dev, Touch detected, start worker thread\n);
+
+disable_irq_nosync(irq);
+
+/* Disable the touch detection plates */
+gpiod_set_value(vf50_ts-gpio_ym, 0);
+
+/* Let the platform mux to default state in order to mux as ADC
*/
+pinctrl_pm_select_default_state(dev);
+
+queue_work(vf50_ts-ts_workqueue, vf50_ts-ts_work);
If you convert this to a threaded interrupt you won't need to
disable/reenable interrupt or queue work. You should also be able to use
gpiod_set_value_cansleep() extending the range of ways the controller
could be connected to systems.
I'm not sure if a threaded interrupt is the right thing here. While the
pen is on the touchscreen (which can be for several seconds)
measurements have to be made in a continuous loop. Is it ok for a
threaded interrupt to run that long?
Yes, why not? Threaded interrupt is simply a kernel thread that is woken
when hard interrupt handler tells it to wake up. Very similar to
interrupt + work queue, except that the kernel manages interactions
properly for you. There are several drivers in kernel that do that, for
example auo-pixcir-ts.c or tsc2007.c
I'm also not sure if it is really safe to _not_ disable the pen down
GPIO interrupt. If we get a interrupt while measuring, we should ignore
that interrupt.
The interrupt management core (you'll have to annotate it as
IRQF_ONESHOT) will make sure it stays masked properly until the threaded
handler completes so you do not need to disable it explicitly.
(snip)
I tried the IRQ threaded implementation. From your reply, I can see my first
implementation was wrong in the sense that I did not use the IRQF_ONESHOT flag.
The touch response time was not good in this case, however thats to be expected
in this case from what I understand now.
With the IRQF_ONESHOT specified the response time is much better compared to
what I was seeing above, but, I still feel it is not the same as with IRQ
handler
plus workqueue approach. However I have no idea how to quantify this.
So I tried explicit enabling/disabling of IRQ and to me it seems the response
slightly improves compared to
Re: [PATCH v2 3/4] touchscreen: colibri-vf50-ts: Add touchscreen support for Colibri VF50
Hello Dmitry,
On 15-07-17 16:42:42, Dmitry Torokhov wrote:
> Hi Sanchayan,
>
>
> On Thu, Jul 16, 2015 at 08:43:21PM +0530, Sanchayan Maity wrote:
> > The Colibri Vybrid VF50 module supports 4-wire touchscreens using
> > FETs and ADC inputs. This driver uses the IIO consumer interface
> > and relies on the vf610_adc driver based on the IIO framework.
>
> This looks pretty good, thank you. Just a few comments below.
>
> >
> > Signed-off-by: Sanchayan Maity
> > ---
> > drivers/input/touchscreen/Kconfig | 12 +
> > drivers/input/touchscreen/Makefile | 1 +
> > drivers/input/touchscreen/colibri-vf50-ts.c | 451
> >
> > 3 files changed, 464 insertions(+)
> > create mode 100644 drivers/input/touchscreen/colibri-vf50-ts.c
> >
> > diff --git a/drivers/input/touchscreen/Kconfig
> > b/drivers/input/touchscreen/Kconfig
> > index 80f6386..28948ca 100644
> > --- a/drivers/input/touchscreen/Kconfig
> > +++ b/drivers/input/touchscreen/Kconfig
> > @@ -1027,4 +1027,16 @@ config TOUCHSCREEN_ZFORCE
> > To compile this driver as a module, choose M here: the
> > module will be called zforce_ts.
> >
> > +config TOUCHSCREEN_COLIBRI_VF50
> > + tristate "Toradex Colibri on board touchscreen driver"
> > + depends on GPIOLIB && IIO && VF610_ADC
> > + help
> > + Say Y here if you have a Colibri VF50 and plan to use
> > + the on-board provided 4-wire touchscreen driver.
> > +
> > + If unsure, say N.
> > +
> > + To compile this driver as a module, choose M here: the
> > + module will be called colibri_vf50_ts.
> > +
> > endif
> > diff --git a/drivers/input/touchscreen/Makefile
> > b/drivers/input/touchscreen/Makefile
> > index 44deea7..93746a0 100644
> > --- a/drivers/input/touchscreen/Makefile
> > +++ b/drivers/input/touchscreen/Makefile
> > @@ -84,3 +84,4 @@ obj-$(CONFIG_TOUCHSCREEN_W90X900) += w90p910_ts.o
> > obj-$(CONFIG_TOUCHSCREEN_SX8654) += sx8654.o
> > obj-$(CONFIG_TOUCHSCREEN_TPS6507X) += tps6507x-ts.o
> > obj-$(CONFIG_TOUCHSCREEN_ZFORCE) += zforce_ts.o
> > +obj-$(CONFIG_TOUCHSCREEN_COLIBRI_VF50) += colibri-vf50-ts.o
> > diff --git a/drivers/input/touchscreen/colibri-vf50-ts.c
> > b/drivers/input/touchscreen/colibri-vf50-ts.c
> > new file mode 100644
> > index 000..eb16bdc
> > --- /dev/null
> > +++ b/drivers/input/touchscreen/colibri-vf50-ts.c
> > @@ -0,0 +1,451 @@
> > +/* Copyright 2015 Toradex AG
> > + *
> > + * Toradex Colibri VF50 Touchscreen driver
> > + *
> > + * Originally authored by Stefan Agner for 3.0 kernel
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > + */
> > +
> > +#include
>
> Why?
Remnant of old usage. Will fix.
>
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
>
> Is not used as far as I can see.
Right. Crept in from old usage of extracting gpio information from DT. Should
have removed when I switched to using gpiod_* and removal of legacy gpio API.
Will fix.
>
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +#define DRIVER_NAME "colibri-vf50-ts"
> > +#define DRV_VERSION "1.0"
> > +
> > +#define VF_ADC_MAX ((1 << 12) - 1)
> > +
> > +#define COLI_TOUCH_MIN_DELAY_US 1000
> > +#define COLI_TOUCH_MAX_DELAY_US 2000
> > +
> > +static int min_pressure = 200;
> > +
> > +struct vf50_touch_device {
> > + struct platform_device *pdev;
> > + struct input_dev*ts_input;
> > + struct workqueue_struct *ts_workqueue;
> > + struct work_struct ts_work;
> > + struct iio_channel *channels;
> > + struct gpio_desc *gpio_xp;
> > + struct gpio_desc *gpio_xm;
> > + struct gpio_desc *gpio_yp;
> > + struct gpio_desc *gpio_ym;
> > + struct gpio_desc *gpio_pen_detect;
> > + struct gpio_desc *gpio_pen_detect_pullup;
> > + int pen_irq;
> > + bool stop_touchscreen;
> > +};
> > +
> > +/*
> > + * Enables given plates and measures touch parameters using ADC
> > + */
> > +static int adc_ts_measure(struct iio_channel *channel,
> > + struct gpio_desc *plate_p, struct gpio_desc *plate_m)
> > +{
> > + int i, value = 0, val = 0;
> > + int ret;
> > +
> > + gpiod_set_value(plate_p, 1);
> > + gpiod_set_value(plate_m, 1);
> > +
> > + usleep_range(COLI_TOUCH_MIN_DELAY_US, COLI_TOUCH_MAX_DELAY_US);
> > +
> > + for (i = 0; i < 5; i++) {
> > + ret = iio_read_channel_raw(channel, );
> > + if (ret < 0)
> > + return -EINVAL;
> > +
> > + value += val;
> > + }
> > +
> > + value /= 5;
> > +
> > + gpiod_set_value(plate_p, 0);
> > + gpiod_set_value(plate_m, 0);
> > +
> > + return value;
> > +}
> > +
> > +/*
> > + * Enable touch detection using falling edge
Re: [PATCH v2 3/4] touchscreen: colibri-vf50-ts: Add touchscreen support for Colibri VF50
On 15-07-18 14:03:25, Nicolae Rosia wrote:
> Hi,
>
> On Thu, Jul 16, 2015 at 6:13 PM, Sanchayan Maity
> wrote:
> > The Colibri Vybrid VF50 module supports 4-wire touchscreens using
> > FETs and ADC inputs. This driver uses the IIO consumer interface
> > and relies on the vf610_adc driver based on the IIO framework.
> >
> > Signed-off-by: Sanch
> > +static const struct of_device_id vf50_touch_of_match[] = {
> > + { .compatible = "toradex,vf50-touchctrl", },
> > + { }
> > +};
> > +MODULE_DEVICE_TABLE(of, vf50_touch_of_match);
> > +
> > +static struct platform_driver __refdata vf50_touch_driver = {
> > + .driver = {
> > + .name = "toradex,vf50_touchctrl",
> > + .of_match_table = vf50_touch_of_match,
> > + },
> > + .probe = vf50_ts_probe,
> > + .remove = vf50_ts_remove,
> > + .prevent_deferred_probe = false,
> > +};
> Why Toradex ? Isn't this a Freescale IP ?
The 4 wire touchscreen support is provided by using on module circuitry
mainly comprising of FET's and leveraging the GPIOs and on chip ADC of
the Vybrid SoC.
This is specific to our Colibri Vybrid VF50 module and not the Freescale
IP. While I guess one could certainly use the driver for their own needs
if one were to replicate a similar circuitry and change the DT properties
concerning GPIO and ADC's as per their own board, as of now this is only
use on our Toradex VF50 modules and was done by us specifically to provide
touchscreen support for VF50.
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: [PATCH v2 3/4] touchscreen: colibri-vf50-ts: Add touchscreen support for Colibri VF50
On 15-07-18 14:03:25, Nicolae Rosia wrote:
Hi,
On Thu, Jul 16, 2015 at 6:13 PM, Sanchayan Maity
maitysancha...@gmail.com wrote:
The Colibri Vybrid VF50 module supports 4-wire touchscreens using
FETs and ADC inputs. This driver uses the IIO consumer interface
and relies on the vf610_adc driver based on the IIO framework.
Signed-off-by: Sanch
+static const struct of_device_id vf50_touch_of_match[] = {
+ { .compatible = toradex,vf50-touchctrl, },
+ { }
+};
+MODULE_DEVICE_TABLE(of, vf50_touch_of_match);
+
+static struct platform_driver __refdata vf50_touch_driver = {
+ .driver = {
+ .name = toradex,vf50_touchctrl,
+ .of_match_table = vf50_touch_of_match,
+ },
+ .probe = vf50_ts_probe,
+ .remove = vf50_ts_remove,
+ .prevent_deferred_probe = false,
+};
Why Toradex ? Isn't this a Freescale IP ?
The 4 wire touchscreen support is provided by using on module circuitry
mainly comprising of FET's and leveraging the GPIOs and on chip ADC of
the Vybrid SoC.
This is specific to our Colibri Vybrid VF50 module and not the Freescale
IP. While I guess one could certainly use the driver for their own needs
if one were to replicate a similar circuitry and change the DT properties
concerning GPIO and ADC's as per their own board, as of now this is only
use on our Toradex VF50 modules and was done by us specifically to provide
touchscreen support for VF50.
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: [PATCH v2 3/4] touchscreen: colibri-vf50-ts: Add touchscreen support for Colibri VF50
Hello Dmitry,
On 15-07-17 16:42:42, Dmitry Torokhov wrote:
Hi Sanchayan,
On Thu, Jul 16, 2015 at 08:43:21PM +0530, Sanchayan Maity wrote:
The Colibri Vybrid VF50 module supports 4-wire touchscreens using
FETs and ADC inputs. This driver uses the IIO consumer interface
and relies on the vf610_adc driver based on the IIO framework.
This looks pretty good, thank you. Just a few comments below.
Signed-off-by: Sanchayan Maity maitysancha...@gmail.com
---
drivers/input/touchscreen/Kconfig | 12 +
drivers/input/touchscreen/Makefile | 1 +
drivers/input/touchscreen/colibri-vf50-ts.c | 451
3 files changed, 464 insertions(+)
create mode 100644 drivers/input/touchscreen/colibri-vf50-ts.c
diff --git a/drivers/input/touchscreen/Kconfig
b/drivers/input/touchscreen/Kconfig
index 80f6386..28948ca 100644
--- a/drivers/input/touchscreen/Kconfig
+++ b/drivers/input/touchscreen/Kconfig
@@ -1027,4 +1027,16 @@ config TOUCHSCREEN_ZFORCE
To compile this driver as a module, choose M here: the
module will be called zforce_ts.
+config TOUCHSCREEN_COLIBRI_VF50
+ tristate Toradex Colibri on board touchscreen driver
+ depends on GPIOLIB IIO VF610_ADC
+ help
+ Say Y here if you have a Colibri VF50 and plan to use
+ the on-board provided 4-wire touchscreen driver.
+
+ If unsure, say N.
+
+ To compile this driver as a module, choose M here: the
+ module will be called colibri_vf50_ts.
+
endif
diff --git a/drivers/input/touchscreen/Makefile
b/drivers/input/touchscreen/Makefile
index 44deea7..93746a0 100644
--- a/drivers/input/touchscreen/Makefile
+++ b/drivers/input/touchscreen/Makefile
@@ -84,3 +84,4 @@ obj-$(CONFIG_TOUCHSCREEN_W90X900) += w90p910_ts.o
obj-$(CONFIG_TOUCHSCREEN_SX8654) += sx8654.o
obj-$(CONFIG_TOUCHSCREEN_TPS6507X) += tps6507x-ts.o
obj-$(CONFIG_TOUCHSCREEN_ZFORCE) += zforce_ts.o
+obj-$(CONFIG_TOUCHSCREEN_COLIBRI_VF50) += colibri-vf50-ts.o
diff --git a/drivers/input/touchscreen/colibri-vf50-ts.c
b/drivers/input/touchscreen/colibri-vf50-ts.c
new file mode 100644
index 000..eb16bdc
--- /dev/null
+++ b/drivers/input/touchscreen/colibri-vf50-ts.c
@@ -0,0 +1,451 @@
+/* Copyright 2015 Toradex AG
+ *
+ * Toradex Colibri VF50 Touchscreen driver
+ *
+ * Originally authored by Stefan Agner for 3.0 kernel
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include dt-bindings/gpio/gpio.h
Why?
Remnant of old usage. Will fix.
+#include linux/delay.h
+#include linux/err.h
+#include linux/gpio.h
+#include linux/iio/consumer.h
+#include linux/iio/types.h
+#include linux/input.h
+#include linux/interrupt.h
+#include linux/kernel.h
+#include linux/module.h
+#include linux/of_gpio.h
Is not used as far as I can see.
Right. Crept in from old usage of extracting gpio information from DT. Should
have removed when I switched to using gpiod_* and removal of legacy gpio API.
Will fix.
+#include linux/pinctrl/consumer.h
+#include linux/platform_device.h
+#include linux/slab.h
+#include linux/types.h
+
+#define DRIVER_NAME colibri-vf50-ts
+#define DRV_VERSION 1.0
+
+#define VF_ADC_MAX ((1 12) - 1)
+
+#define COLI_TOUCH_MIN_DELAY_US 1000
+#define COLI_TOUCH_MAX_DELAY_US 2000
+
+static int min_pressure = 200;
+
+struct vf50_touch_device {
+ struct platform_device *pdev;
+ struct input_dev*ts_input;
+ struct workqueue_struct *ts_workqueue;
+ struct work_struct ts_work;
+ struct iio_channel *channels;
+ struct gpio_desc *gpio_xp;
+ struct gpio_desc *gpio_xm;
+ struct gpio_desc *gpio_yp;
+ struct gpio_desc *gpio_ym;
+ struct gpio_desc *gpio_pen_detect;
+ struct gpio_desc *gpio_pen_detect_pullup;
+ int pen_irq;
+ bool stop_touchscreen;
+};
+
+/*
+ * Enables given plates and measures touch parameters using ADC
+ */
+static int adc_ts_measure(struct iio_channel *channel,
+ struct gpio_desc *plate_p, struct gpio_desc *plate_m)
+{
+ int i, value = 0, val = 0;
+ int ret;
+
+ gpiod_set_value(plate_p, 1);
+ gpiod_set_value(plate_m, 1);
+
+ usleep_range(COLI_TOUCH_MIN_DELAY_US, COLI_TOUCH_MAX_DELAY_US);
+
+ for (i = 0; i 5; i++) {
+ ret = iio_read_channel_raw(channel, val);
+ if (ret 0)
+ return -EINVAL;
+
+ value += val;
+ }
+
+ value /= 5;
+
+ gpiod_set_value(plate_p, 0);
+ gpiod_set_value(plate_m, 0);
+
+ return value;
+}
+
+/*
+ * Enable touch detection using falling edge detection on XM
+ */
+static
Re: [PATCH v2 2/2] ARM: dts: vfxxx: Add property for minimum sample time
Hello Jonathan,
On 15-07-11 18:39:10, Jonathan Cameron wrote:
> On 10/07/15 19:06, maitysancha...@gmail.com wrote:
> > Hello Shawn,
> >
> > On 15-07-10 16:53:24, Shawn Guo wrote:
> >> On Wed, Jun 24, 2015 at 02:03:41PM +0530, Sanchayan Maity wrote:
> >>> Add a device tree property which allows to specify the minimum sample
> >>> time which can be used to calculate the actual ADC cycles required
> >>> depending on the hardware.
> >>>
> >>> Signed-off-by: Sanchayan Maity
> >>> ---
> >>> arch/arm/boot/dts/vfxxx.dtsi | 2 ++
> >>> 1 file changed, 2 insertions(+)
> >>>
> >>> diff --git a/arch/arm/boot/dts/vfxxx.dtsi b/arch/arm/boot/dts/vfxxx.dtsi
> >>> index 90a03d5..71d9c08 100644
> >>> --- a/arch/arm/boot/dts/vfxxx.dtsi
> >>> +++ b/arch/arm/boot/dts/vfxxx.dtsi
> >>> @@ -229,6 +229,7 @@
> >>> status = "disabled";
> >>> fsl,adck-max-frequency = <3000>, <4000>,
> >>> <2000>;
> >>> + min-sample-time = <1000>;
> >>> };
> >>>
> >>> wdoga5: wdog@4003e000 {
> >>> @@ -447,6 +448,7 @@
> >>> status = "disabled";
> >>> fsl,adck-max-frequency = <3000>, <4000>,
> >>> <2000>;
> >>> + min-sample-time = <1000>;
> >>
> >> Can we code 1000 as the default in kernel driver, so that only boards
> >> requiring different value need to have this property? Doing so makes
> >> the property optional rather than required.
> >>
> >
> > Not sure if hardcoding it in the driver is the right approach.
> If it is a true feature of the device (i.e. if in the case of perfect
> front end electronics) this is the right option, then a default makes
> a lot of sense. If that isn't the case (I suspect not) then if we
> drop it be optional chances are no one will bother thinking about it
> or trying to tune this at all.
>
> Hence seems wrong to put a fairly arbitrary default value on it.
> However, we do need to still work with old device trees and new kernels
> so need to cope without it.
>
> Hence to my mind, if we had started out with this in the first driver
> version, then the default would be a bad idea. As we didn't then we
> really need to cope with nothing specified (as best we can) and so
> we do need a sensible default (or perhaps even sensible worst
> case default) in there.
Just to be sure, do I understand you correctly that you agree with the
property being in device tree?
If the device tree property is not specified the driver will just go on
to use the value "3" which was hardcoded earlier. In my opinion it is
better to allow users to change the sampling cycles by specifying or not
specifying this in the device tree rather than have a default value coded
in the driver. However this is just my opinion.
Also, some users might not need the somewhat worst case value of 1000. I
guess we could keep the driver patch the way it is right now and migrate
the property to be specified in our board dts file? So the property can
be picked up from the vf-colibri.dtsi or vf500-colibri.dtsi in the adc
node? Other boards can do the same?
We came up with the change after noticing huge reading discrepancies where
we had a 4 wire resistive touch screen connected to the ADC channels and
the driver sampled these channels at an interval of 10-20ms[1]. Once the
touchscreen came into picture, readings from temperature channel or others
showed deviations between 4-6. Somehow the temperature channel
seemed to be the most affected.
For a while, I thought the ts driver logic was at a fault, but Stefan pointed
out the discrepancies in the driver using a fixed clock cycle which was not
correct along the sampling time also being hardcoded. Stefan's "respect ADC
clocking limitations" and this patch are based on our above observations.
[1] https://lkml.org/lkml/2015/6/30/103
- 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: [PATCH v2 2/2] ARM: dts: vfxxx: Add property for minimum sample time
Hello Jonathan,
On 15-07-11 18:39:10, Jonathan Cameron wrote:
On 10/07/15 19:06, maitysancha...@gmail.com wrote:
Hello Shawn,
On 15-07-10 16:53:24, Shawn Guo wrote:
On Wed, Jun 24, 2015 at 02:03:41PM +0530, Sanchayan Maity wrote:
Add a device tree property which allows to specify the minimum sample
time which can be used to calculate the actual ADC cycles required
depending on the hardware.
Signed-off-by: Sanchayan Maity maitysancha...@gmail.com
---
arch/arm/boot/dts/vfxxx.dtsi | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arm/boot/dts/vfxxx.dtsi b/arch/arm/boot/dts/vfxxx.dtsi
index 90a03d5..71d9c08 100644
--- a/arch/arm/boot/dts/vfxxx.dtsi
+++ b/arch/arm/boot/dts/vfxxx.dtsi
@@ -229,6 +229,7 @@
status = disabled;
fsl,adck-max-frequency = 3000, 4000,
2000;
+ min-sample-time = 1000;
};
wdoga5: wdog@4003e000 {
@@ -447,6 +448,7 @@
status = disabled;
fsl,adck-max-frequency = 3000, 4000,
2000;
+ min-sample-time = 1000;
Can we code 1000 as the default in kernel driver, so that only boards
requiring different value need to have this property? Doing so makes
the property optional rather than required.
Not sure if hardcoding it in the driver is the right approach.
If it is a true feature of the device (i.e. if in the case of perfect
front end electronics) this is the right option, then a default makes
a lot of sense. If that isn't the case (I suspect not) then if we
drop it be optional chances are no one will bother thinking about it
or trying to tune this at all.
Hence seems wrong to put a fairly arbitrary default value on it.
However, we do need to still work with old device trees and new kernels
so need to cope without it.
Hence to my mind, if we had started out with this in the first driver
version, then the default would be a bad idea. As we didn't then we
really need to cope with nothing specified (as best we can) and so
we do need a sensible default (or perhaps even sensible worst
case default) in there.
Just to be sure, do I understand you correctly that you agree with the
property being in device tree?
If the device tree property is not specified the driver will just go on
to use the value 3 which was hardcoded earlier. In my opinion it is
better to allow users to change the sampling cycles by specifying or not
specifying this in the device tree rather than have a default value coded
in the driver. However this is just my opinion.
Also, some users might not need the somewhat worst case value of 1000. I
guess we could keep the driver patch the way it is right now and migrate
the property to be specified in our board dts file? So the property can
be picked up from the vf-colibri.dtsi or vf500-colibri.dtsi in the adc
node? Other boards can do the same?
We came up with the change after noticing huge reading discrepancies where
we had a 4 wire resistive touch screen connected to the ADC channels and
the driver sampled these channels at an interval of 10-20ms[1]. Once the
touchscreen came into picture, readings from temperature channel or others
showed deviations between 4-6. Somehow the temperature channel
seemed to be the most affected.
For a while, I thought the ts driver logic was at a fault, but Stefan pointed
out the discrepancies in the driver using a fixed clock cycle which was not
correct along the sampling time also being hardcoded. Stefan's respect ADC
clocking limitations and this patch are based on our above observations.
[1] https://lkml.org/lkml/2015/6/30/103
- 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: [PATCH v2 2/2] ARM: dts: vfxxx: Add property for minimum sample time
Hello Shawn,
On 15-07-10 16:53:24, Shawn Guo wrote:
> On Wed, Jun 24, 2015 at 02:03:41PM +0530, Sanchayan Maity wrote:
> > Add a device tree property which allows to specify the minimum sample
> > time which can be used to calculate the actual ADC cycles required
> > depending on the hardware.
> >
> > Signed-off-by: Sanchayan Maity
> > ---
> > arch/arm/boot/dts/vfxxx.dtsi | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/arch/arm/boot/dts/vfxxx.dtsi b/arch/arm/boot/dts/vfxxx.dtsi
> > index 90a03d5..71d9c08 100644
> > --- a/arch/arm/boot/dts/vfxxx.dtsi
> > +++ b/arch/arm/boot/dts/vfxxx.dtsi
> > @@ -229,6 +229,7 @@
> > status = "disabled";
> > fsl,adck-max-frequency = <3000>, <4000>,
> > <2000>;
> > + min-sample-time = <1000>;
> > };
> >
> > wdoga5: wdog@4003e000 {
> > @@ -447,6 +448,7 @@
> > status = "disabled";
> > fsl,adck-max-frequency = <3000>, <4000>,
> > <2000>;
> > + min-sample-time = <1000>;
>
> Can we code 1000 as the default in kernel driver, so that only boards
> requiring different value need to have this property? Doing so makes
> the property optional rather than required.
>
Not sure if hardcoding it in the driver is the right approach.
However if the maintainers and others agree on doing this, I will do
the necessary change.
Thanks.
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: [PATCH v2 0/2] Implement sample time consideration for Vybrid's ADC
Hello Shawn, On 15-07-10 16:47:04, Shawn Guo wrote: > On Wed, Jun 24, 2015 at 02:03:39PM +0530, Sanchayan Maity wrote: > > Hello, > > > > This patchset adds a dt binding for specifying sample time > > for the vybrid adc driver and takes this into account for > > sampling frequency calculation and related configuration in > > the driver. > > > > The patchset is based on top of Stefan's patches here > > http://lkml.iu.edu/hypermail/linux/kernel/1505.3/02043.html > > > > which got recently applied. Tested with shawn's for-next > > branch. > > > > Changes since v1: > > > > Change from a vendor specific fsl,min-sample-time to non vendor > > specific min-sample-time. > > What's the reason for that? Property without vendor prefix would be > the generic one, which should be defined by generic ADC bindings, not > vf610-adc.txt. The reason for going with a generic one was discussed in the first series, as though there might not be any devices at the moment using this property, some might appear or use it in the future. I did consider putting this in the generic ADC bindings, however for lack of one like one present for touchscreen, I kept it in the vf610 specific driver binding. Thought there should not be any harm with this. - Sanchayan. > > Shawn > > > > > Version 1 of the patchset can be found here > > http://lkml.iu.edu/hypermail/linux/kernel/1506.1/00026.html > > > > - Sanchayan. > > > > Sanchayan Maity (2): > > iio: adc: Determine sampling frequencies by using minimum sample time > > ARM: dts: vfxxx: Add property for minimum sample time > > > > .../devicetree/bindings/iio/adc/vf610-adc.txt | 6 ++ > > arch/arm/boot/dts/vfxxx.dtsi | 2 + > > drivers/iio/adc/vf610_adc.c| 74 > > -- > > 3 files changed, 78 insertions(+), 4 deletions(-) > > > > -- > > 2.4.4 > > > > -- > > 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/ > > -- 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: [PATCH v2 0/2] Implement sample time consideration for Vybrid's ADC
Hello Shawn, On 15-07-10 16:47:04, Shawn Guo wrote: On Wed, Jun 24, 2015 at 02:03:39PM +0530, Sanchayan Maity wrote: Hello, This patchset adds a dt binding for specifying sample time for the vybrid adc driver and takes this into account for sampling frequency calculation and related configuration in the driver. The patchset is based on top of Stefan's patches here http://lkml.iu.edu/hypermail/linux/kernel/1505.3/02043.html which got recently applied. Tested with shawn's for-next branch. Changes since v1: Change from a vendor specific fsl,min-sample-time to non vendor specific min-sample-time. What's the reason for that? Property without vendor prefix would be the generic one, which should be defined by generic ADC bindings, not vf610-adc.txt. The reason for going with a generic one was discussed in the first series, as though there might not be any devices at the moment using this property, some might appear or use it in the future. I did consider putting this in the generic ADC bindings, however for lack of one like one present for touchscreen, I kept it in the vf610 specific driver binding. Thought there should not be any harm with this. - Sanchayan. Shawn Version 1 of the patchset can be found here http://lkml.iu.edu/hypermail/linux/kernel/1506.1/00026.html - Sanchayan. Sanchayan Maity (2): iio: adc: Determine sampling frequencies by using minimum sample time ARM: dts: vfxxx: Add property for minimum sample time .../devicetree/bindings/iio/adc/vf610-adc.txt | 6 ++ arch/arm/boot/dts/vfxxx.dtsi | 2 + drivers/iio/adc/vf610_adc.c| 74 -- 3 files changed, 78 insertions(+), 4 deletions(-) -- 2.4.4 -- 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/ -- 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: [PATCH v2 2/2] ARM: dts: vfxxx: Add property for minimum sample time
Hello Shawn,
On 15-07-10 16:53:24, Shawn Guo wrote:
On Wed, Jun 24, 2015 at 02:03:41PM +0530, Sanchayan Maity wrote:
Add a device tree property which allows to specify the minimum sample
time which can be used to calculate the actual ADC cycles required
depending on the hardware.
Signed-off-by: Sanchayan Maity maitysancha...@gmail.com
---
arch/arm/boot/dts/vfxxx.dtsi | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arm/boot/dts/vfxxx.dtsi b/arch/arm/boot/dts/vfxxx.dtsi
index 90a03d5..71d9c08 100644
--- a/arch/arm/boot/dts/vfxxx.dtsi
+++ b/arch/arm/boot/dts/vfxxx.dtsi
@@ -229,6 +229,7 @@
status = disabled;
fsl,adck-max-frequency = 3000, 4000,
2000;
+ min-sample-time = 1000;
};
wdoga5: wdog@4003e000 {
@@ -447,6 +448,7 @@
status = disabled;
fsl,adck-max-frequency = 3000, 4000,
2000;
+ min-sample-time = 1000;
Can we code 1000 as the default in kernel driver, so that only boards
requiring different value need to have this property? Doing so makes
the property optional rather than required.
Not sure if hardcoding it in the driver is the right approach.
However if the maintainers and others agree on doing this, I will do
the necessary change.
Thanks.
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 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 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
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) {
+
Re: [RFC PATCH v6 2/2] nvmem: Add Vybrid OCOTP and OCROM support
Hello Stefan, On 15-06-24 13:52:28, Stefan Wahren wrote: > Hi Srinivas, > > Am 24.06.2015 um 12:45 schrieb maitysancha...@gmail.com: > > Hello Stefan, > > > > On 15-06-24 11:56:14, Stefan Wahren wrote: > >> Hi Sanchayan, > >> > >> Am 24.06.2015 um 07:19 schrieb maitysancha...@gmail.com: > >>> On 15-06-23 21:31:41, Stefan Wahren wrote: > Hi Sanchayan, > > > Sanchayan Maity hat am 23. Juni 2015 um 15:44 > > geschrieben: > > > > > > The patch adds support for the On Chip One Time Programmable Peripheral > > (OCOTP) and On Chip ROM (OCROM) support. > > > > On Vybrid OCOTP contain data like SoC ID, MAC address and OCROM has the > > revision ID. > > > > Signed-off-by: Sanchayan Maity > > --- > > drivers/nvmem/Kconfig | 11 + > > drivers/nvmem/Makefile | 2 ++ > > drivers/nvmem/vf610-ocotp.c | 60 > > + > > 3 files changed, 73 insertions(+) > > create mode 100644 drivers/nvmem/vf610-ocotp.c > > > > diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig > > index 17f1a57..557c1e0 100644 > > --- a/drivers/nvmem/Kconfig > > +++ b/drivers/nvmem/Kconfig > > @@ -33,4 +33,15 @@ 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 > > + select REGMAP_MMIO > how do you come to the conclusion that Vybrid On-Chip OTP is accessable > via > MMIO? > >>> Frankly speaking I just changed the naming conventions and followed the > >>> qfrom > >>> and sunxi sid examples in Srinivas's patches. > >>> > >>> I just tested it without the "select REGMAP_MMIO" and it works just fine. > >>> > >>> - Sanchayan. > >> sorry for the confusion. My question refers to the whole driver > >> implementation not only to the REGMAP_MMIO. > >> > >> According to > >> > >> Vybrid Reference Manual F-Series > >> Document Number: VYBRIDRM > >> Rev 7, 06/2014 > >> > >> 35.5 OCOTP memory map/register definition > >> > >> the memory region is organized in control and shadow registers. I'm very > >> sceptical that using REGMAP_MMIO is the right way for accessing the OCOTP. > > Sorry I am not well versed with the regmap stuff. Can you please tell me why > > REGMAP_MMIO is not the right way for accessing the OCOTP? > > i think the implementation of OCOTP driver should be more like this: > > https://git.linaro.org/people/srinivas.kandagatla/linux.git/commit/b4c3ad253747767511233687436f20144e850d67 > > It uses REGMAP with a specialized read function. Thank you very much for the link. > > > > > If this is not the right way, I assume you are referring to the procedures > > in section 35.3.1.5 and 35.3.1.6 for reading and writing from these areas? > > Yes. I think writing isn't needed. But the read function should check > at least for OCOTP_CTRL[BUSY] and OCOTP_CTRL[ERROR]. If one of the > bits is set, the read function should return with an error. Understood. Will incoporate this in the v6 patch. > > This is the same behavior of my OCOTP driver for MXS platform. > > Unfortunately i haven't push the driver to my github account. > > >> It possible that it works in your case. But in the case the lock bits > >> are set the driver won't work correctly. > > As such the previous implementations were very different. > > > > Currently I only expect to provide a way for users to read the SoC ID from > > the OCOTP block. My understanding was even if the lock bits are set, reading > > from the registers will return 0xBADABADA. > > > > For example, currently for three locations I see this from ocotp block > > > > * > > 080 bada bada bada bada bada bada bada bada > > * > > 500 bada bada bada bada bada bada bada bada > > * > > c80 bada bada bada bada bada bada bada bada > > * > > > > - Sanchayan. > > Until somebody comes to the idea to fetch the MAC address from the OCOTP > block ... > > How about doing it right at this point, instead of fixing it later? Of course. Thanks for the feedback Stefan. - 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 2/2] nvmem: Add Vybrid OCOTP and OCROM support
Hello Stefan, On 15-06-24 11:56:14, Stefan Wahren wrote: > Hi Sanchayan, > > Am 24.06.2015 um 07:19 schrieb maitysancha...@gmail.com: > > On 15-06-23 21:31:41, Stefan Wahren wrote: > >> Hi Sanchayan, > >> > >>> Sanchayan Maity hat am 23. Juni 2015 um 15:44 > >>> geschrieben: > >>> > >>> > >>> The patch adds support for the On Chip One Time Programmable Peripheral > >>> (OCOTP) and On Chip ROM (OCROM) support. > >>> > >>> On Vybrid OCOTP contain data like SoC ID, MAC address and OCROM has the > >>> revision ID. > >>> > >>> Signed-off-by: Sanchayan Maity > >>> --- > >>> drivers/nvmem/Kconfig | 11 + > >>> drivers/nvmem/Makefile | 2 ++ > >>> drivers/nvmem/vf610-ocotp.c | 60 > >>> + > >>> 3 files changed, 73 insertions(+) > >>> create mode 100644 drivers/nvmem/vf610-ocotp.c > >>> > >>> diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig > >>> index 17f1a57..557c1e0 100644 > >>> --- a/drivers/nvmem/Kconfig > >>> +++ b/drivers/nvmem/Kconfig > >>> @@ -33,4 +33,15 @@ 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 > >>> + select REGMAP_MMIO > >> how do you come to the conclusion that Vybrid On-Chip OTP is accessable via > >> MMIO? > > Frankly speaking I just changed the naming conventions and followed the > > qfrom > > and sunxi sid examples in Srinivas's patches. > > > > I just tested it without the "select REGMAP_MMIO" and it works just fine. > > > > - Sanchayan. > > sorry for the confusion. My question refers to the whole driver > implementation not only to the REGMAP_MMIO. > > According to > > Vybrid Reference Manual F-Series > Document Number: VYBRIDRM > Rev 7, 06/2014 > > 35.5 OCOTP memory map/register definition > > the memory region is organized in control and shadow registers. I'm very > sceptical that using REGMAP_MMIO is the right way for accessing the OCOTP. Sorry I am not well versed with the regmap stuff. Can you please tell me why REGMAP_MMIO is not the right way for accessing the OCOTP? If this is not the right way, I assume you are referring to the procedures in section 35.3.1.5 and 35.3.1.6 for reading and writing from these areas? > > It possible that it works in your case. But in the case the lock bits > are set the driver won't work correctly. As such the previous implementations were very different. Currently I only expect to provide a way for users to read the SoC ID from the OCOTP block. My understanding was even if the lock bits are set, reading from the registers will return 0xBADABADA. For example, currently for three locations I see this from ocotp block * 080 bada bada bada bada bada bada bada bada * 500 bada bada bada bada bada bada bada bada * c80 bada bada bada bada bada bada bada bada * - 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 2/2] nvmem: Add Vybrid OCOTP and OCROM support
Hello Stefan, On 15-06-24 11:56:14, Stefan Wahren wrote: Hi Sanchayan, Am 24.06.2015 um 07:19 schrieb maitysancha...@gmail.com: On 15-06-23 21:31:41, Stefan Wahren wrote: Hi Sanchayan, Sanchayan Maity maitysancha...@gmail.com hat am 23. Juni 2015 um 15:44 geschrieben: The patch adds support for the On Chip One Time Programmable Peripheral (OCOTP) and On Chip ROM (OCROM) support. On Vybrid OCOTP contain data like SoC ID, MAC address and OCROM has the revision ID. Signed-off-by: Sanchayan Maity maitysancha...@gmail.com --- drivers/nvmem/Kconfig | 11 + drivers/nvmem/Makefile | 2 ++ drivers/nvmem/vf610-ocotp.c | 60 + 3 files changed, 73 insertions(+) create mode 100644 drivers/nvmem/vf610-ocotp.c diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig index 17f1a57..557c1e0 100644 --- a/drivers/nvmem/Kconfig +++ b/drivers/nvmem/Kconfig @@ -33,4 +33,15 @@ 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 + select REGMAP_MMIO how do you come to the conclusion that Vybrid On-Chip OTP is accessable via MMIO? Frankly speaking I just changed the naming conventions and followed the qfrom and sunxi sid examples in Srinivas's patches. I just tested it without the select REGMAP_MMIO and it works just fine. - Sanchayan. sorry for the confusion. My question refers to the whole driver implementation not only to the REGMAP_MMIO. According to Vybrid Reference Manual F-Series Document Number: VYBRIDRM Rev 7, 06/2014 35.5 OCOTP memory map/register definition the memory region is organized in control and shadow registers. I'm very sceptical that using REGMAP_MMIO is the right way for accessing the OCOTP. Sorry I am not well versed with the regmap stuff. Can you please tell me why REGMAP_MMIO is not the right way for accessing the OCOTP? If this is not the right way, I assume you are referring to the procedures in section 35.3.1.5 and 35.3.1.6 for reading and writing from these areas? It possible that it works in your case. But in the case the lock bits are set the driver won't work correctly. As such the previous implementations were very different. Currently I only expect to provide a way for users to read the SoC ID from the OCOTP block. My understanding was even if the lock bits are set, reading from the registers will return 0xBADABADA. For example, currently for three locations I see this from ocotp block * 080 bada bada bada bada bada bada bada bada * 500 bada bada bada bada bada bada bada bada * c80 bada bada bada bada bada bada bada bada * - 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 2/2] nvmem: Add Vybrid OCOTP and OCROM support
Hello Stefan, On 15-06-24 13:52:28, Stefan Wahren wrote: Hi Srinivas, Am 24.06.2015 um 12:45 schrieb maitysancha...@gmail.com: Hello Stefan, On 15-06-24 11:56:14, Stefan Wahren wrote: Hi Sanchayan, Am 24.06.2015 um 07:19 schrieb maitysancha...@gmail.com: On 15-06-23 21:31:41, Stefan Wahren wrote: Hi Sanchayan, Sanchayan Maity maitysancha...@gmail.com hat am 23. Juni 2015 um 15:44 geschrieben: The patch adds support for the On Chip One Time Programmable Peripheral (OCOTP) and On Chip ROM (OCROM) support. On Vybrid OCOTP contain data like SoC ID, MAC address and OCROM has the revision ID. Signed-off-by: Sanchayan Maity maitysancha...@gmail.com --- drivers/nvmem/Kconfig | 11 + drivers/nvmem/Makefile | 2 ++ drivers/nvmem/vf610-ocotp.c | 60 + 3 files changed, 73 insertions(+) create mode 100644 drivers/nvmem/vf610-ocotp.c diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig index 17f1a57..557c1e0 100644 --- a/drivers/nvmem/Kconfig +++ b/drivers/nvmem/Kconfig @@ -33,4 +33,15 @@ 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 + select REGMAP_MMIO how do you come to the conclusion that Vybrid On-Chip OTP is accessable via MMIO? Frankly speaking I just changed the naming conventions and followed the qfrom and sunxi sid examples in Srinivas's patches. I just tested it without the select REGMAP_MMIO and it works just fine. - Sanchayan. sorry for the confusion. My question refers to the whole driver implementation not only to the REGMAP_MMIO. According to Vybrid Reference Manual F-Series Document Number: VYBRIDRM Rev 7, 06/2014 35.5 OCOTP memory map/register definition the memory region is organized in control and shadow registers. I'm very sceptical that using REGMAP_MMIO is the right way for accessing the OCOTP. Sorry I am not well versed with the regmap stuff. Can you please tell me why REGMAP_MMIO is not the right way for accessing the OCOTP? i think the implementation of OCOTP driver should be more like this: https://git.linaro.org/people/srinivas.kandagatla/linux.git/commit/b4c3ad253747767511233687436f20144e850d67 It uses REGMAP with a specialized read function. Thank you very much for the link. If this is not the right way, I assume you are referring to the procedures in section 35.3.1.5 and 35.3.1.6 for reading and writing from these areas? Yes. I think writing isn't needed. But the read function should check at least for OCOTP_CTRL[BUSY] and OCOTP_CTRL[ERROR]. If one of the bits is set, the read function should return with an error. Understood. Will incoporate this in the v6 patch. This is the same behavior of my OCOTP driver for MXS platform. Unfortunately i haven't push the driver to my github account. It possible that it works in your case. But in the case the lock bits are set the driver won't work correctly. As such the previous implementations were very different. Currently I only expect to provide a way for users to read the SoC ID from the OCOTP block. My understanding was even if the lock bits are set, reading from the registers will return 0xBADABADA. For example, currently for three locations I see this from ocotp block * 080 bada bada bada bada bada bada bada bada * 500 bada bada bada bada bada bada bada bada * c80 bada bada bada bada bada bada bada bada * - Sanchayan. Until somebody comes to the idea to fetch the MAC address from the OCOTP block ... How about doing it right at this point, instead of fixing it later? Of course. Thanks for the feedback Stefan. - 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 2/2] nvmem: Add Vybrid OCOTP and OCROM support
Hello Srinivas,
On 15-06-23 20:03:31, Srinivas Kandagatla wrote:
> Hi Sanchayan,
>
>
> On 23/06/15 14:44, Sanchayan Maity wrote:
> >The patch adds support for the On Chip One Time Programmable Peripheral
> >(OCOTP) and On Chip ROM (OCROM) support.
> >
> >On Vybrid OCOTP contain data like SoC ID, MAC address and OCROM has the
> >revision ID.
> >
> >Signed-off-by: Sanchayan Maity
> >---
> > drivers/nvmem/Kconfig | 11 +
> > drivers/nvmem/Makefile | 2 ++
> > drivers/nvmem/vf610-ocotp.c | 60
> > +
> > 3 files changed, 73 insertions(+)
> > create mode 100644 drivers/nvmem/vf610-ocotp.c
> >
>
> Fantastic! diff is already looking good, when compared to v5 patches,
> "drivers/soc/fsl/soc-vf610.c | 166
> +++"
Yes :)
>
> I think, We could even do a better job by moving qfprom and this driver to a
> simple-mmio-nvmem provider, see below comments.
>
> >diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig
> >index 17f1a57..557c1e0 100644
> >--- a/drivers/nvmem/Kconfig
> >+++ b/drivers/nvmem/Kconfig
> >@@ -33,4 +33,15 @@ 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
> >+select REGMAP_MMIO
> >+help
> >+ This is a driver for the 'OCOTP' available on various Vybrid
> >+ devices.
> >+
> >+ 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..d98772d
> >--- /dev/null
> >+++ b/drivers/nvmem/vf610-ocotp.c
> >@@ -0,0 +1,60 @@
> >+/*
> >+ * 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 "nvmem-mmio.h"
> >+
> >+static struct regmap_config regmap_config = {
> >+.reg_bits = 32,
> >+.val_bits = 32,
> >+.reg_stride = 4,
>
> Any particular reason why it cant support byte reads?
>
> This driver looks exactly same as qcom,qfprom driver, except the
> regmap_config.
It does support byte reads. This works as well
static struct regmap_config regmap_config = {
.reg_bits = 32,
.val_bits = 8,
.reg_stride = 1,
};
}
Hmm can't recall why I went with that. Yes it is more or less the
same as qcom,qfprom driver, that's what I referred.
>
> >+};
> >+
> >+static struct nvmem_config ocotp_config = {
> >+.name = "soc_id",
> >+};
> >+
> >+static struct nvmem_config rom_config = {
> >+.name = "rom_rev",
> >+};
> >+
> >+static struct nvmem_mmio_data ocotp_data = {
> >+.nvmem_config = _config,
> >+.regmap_config = _config,
> >+};
> >+
> >+static struct nvmem_mmio_data rom_data = {
> >+.nvmem_config = _config,
> >+.regmap_config = _config,
> >+};
> >+
> >+static const struct of_device_id ocotp_of_match[] = {
> >+{ .compatible = "fsl,vf610-ocotp", .data = _data},
> >+{ .compatible = "fsl,vf610-ocrom", .data = _data},
> >+{/* sentinel */},
> >+};
> >+MODULE_DEVICE_TABLE(of, ocotp_of_match);
> >+
> >+static struct platform_driver vf610_ocotp_driver = {
> >+.probe = nvmem_mmio_probe,
> >+.remove = nvmem_mmio_remove,
> >+.driver = {
> >+.name = "vf610-nvmem",
> >+.of_match_table = ocotp_of_match,
> >+},
> >+};
> >+module_platform_driver(vf610_ocotp_driver);
> >+MODULE_AUTHOR("Sanchayan Maity ");
> >+MODULE_DESCRIPTION("Vybrid NVMEM driver");
> >+MODULE_LICENSE("GPL v2");
> >
>
>
> I moved the nvmem_mmio to qfprom as I did not any other user for that in v6
> patches.
>
> Now that Vybrid can be another user I should probably put it back, or may be
> I should create a simple-mmio-nvmem driver which both qcom,qfprom and Vybrid
> can use.
Nice, then that would simplify things further from what I
Re: [RFC PATCH v6 2/2] nvmem: Add Vybrid OCOTP and OCROM support
On 15-06-23 21:31:41, Stefan Wahren wrote: > Hi Sanchayan, > > > Sanchayan Maity hat am 23. Juni 2015 um 15:44 > > geschrieben: > > > > > > The patch adds support for the On Chip One Time Programmable Peripheral > > (OCOTP) and On Chip ROM (OCROM) support. > > > > On Vybrid OCOTP contain data like SoC ID, MAC address and OCROM has the > > revision ID. > > > > Signed-off-by: Sanchayan Maity > > --- > > drivers/nvmem/Kconfig | 11 + > > drivers/nvmem/Makefile | 2 ++ > > drivers/nvmem/vf610-ocotp.c | 60 > > + > > 3 files changed, 73 insertions(+) > > create mode 100644 drivers/nvmem/vf610-ocotp.c > > > > diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig > > index 17f1a57..557c1e0 100644 > > --- a/drivers/nvmem/Kconfig > > +++ b/drivers/nvmem/Kconfig > > @@ -33,4 +33,15 @@ 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 > > + select REGMAP_MMIO > > how do you come to the conclusion that Vybrid On-Chip OTP is accessable via > MMIO? Frankly speaking I just changed the naming conventions and followed the qfrom and sunxi sid examples in Srinivas's patches. I just tested it without the "select REGMAP_MMIO" and it works just fine. - 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 2/2] nvmem: Add Vybrid OCOTP and OCROM support
Hello Srinivas,
On 15-06-23 20:03:31, Srinivas Kandagatla wrote:
Hi Sanchayan,
On 23/06/15 14:44, Sanchayan Maity wrote:
The patch adds support for the On Chip One Time Programmable Peripheral
(OCOTP) and On Chip ROM (OCROM) support.
On Vybrid OCOTP contain data like SoC ID, MAC address and OCROM has the
revision ID.
Signed-off-by: Sanchayan Maity maitysancha...@gmail.com
---
drivers/nvmem/Kconfig | 11 +
drivers/nvmem/Makefile | 2 ++
drivers/nvmem/vf610-ocotp.c | 60
+
3 files changed, 73 insertions(+)
create mode 100644 drivers/nvmem/vf610-ocotp.c
Fantastic! diff is already looking good, when compared to v5 patches,
drivers/soc/fsl/soc-vf610.c | 166
+++
Yes :)
I think, We could even do a better job by moving qfprom and this driver to a
simple-mmio-nvmem provider, see below comments.
diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig
index 17f1a57..557c1e0 100644
--- a/drivers/nvmem/Kconfig
+++ b/drivers/nvmem/Kconfig
@@ -33,4 +33,15 @@ 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
+select REGMAP_MMIO
+help
+ This is a driver for the 'OCOTP' available on various Vybrid
+ devices.
+
+ 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..d98772d
--- /dev/null
+++ b/drivers/nvmem/vf610-ocotp.c
@@ -0,0 +1,60 @@
+/*
+ * 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/module.h
+#include linux/of.h
+#include nvmem-mmio.h
+
+static struct regmap_config regmap_config = {
+.reg_bits = 32,
+.val_bits = 32,
+.reg_stride = 4,
Any particular reason why it cant support byte reads?
This driver looks exactly same as qcom,qfprom driver, except the
regmap_config.
It does support byte reads. This works as well
static struct regmap_config regmap_config = {
.reg_bits = 32,
.val_bits = 8,
.reg_stride = 1,
};
}
Hmm can't recall why I went with that. Yes it is more or less the
same as qcom,qfprom driver, that's what I referred.
+};
+
+static struct nvmem_config ocotp_config = {
+.name = soc_id,
+};
+
+static struct nvmem_config rom_config = {
+.name = rom_rev,
+};
+
+static struct nvmem_mmio_data ocotp_data = {
+.nvmem_config = ocotp_config,
+.regmap_config = regmap_config,
+};
+
+static struct nvmem_mmio_data rom_data = {
+.nvmem_config = rom_config,
+.regmap_config = regmap_config,
+};
+
+static const struct of_device_id ocotp_of_match[] = {
+{ .compatible = fsl,vf610-ocotp, .data = ocotp_data},
+{ .compatible = fsl,vf610-ocrom, .data = rom_data},
+{/* sentinel */},
+};
+MODULE_DEVICE_TABLE(of, ocotp_of_match);
+
+static struct platform_driver vf610_ocotp_driver = {
+.probe = nvmem_mmio_probe,
+.remove = nvmem_mmio_remove,
+.driver = {
+.name = vf610-nvmem,
+.of_match_table = ocotp_of_match,
+},
+};
+module_platform_driver(vf610_ocotp_driver);
+MODULE_AUTHOR(Sanchayan Maity sanchayan.ma...@toradex.com);
+MODULE_DESCRIPTION(Vybrid NVMEM driver);
+MODULE_LICENSE(GPL v2);
I moved the nvmem_mmio to qfprom as I did not any other user for that in v6
patches.
Now that Vybrid can be another user I should probably put it back, or may be
I should create a simple-mmio-nvmem driver which both qcom,qfprom and Vybrid
can use.
Nice, then that would simplify things further from what I understand at the
moment.
- Sanchayan.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to
Re: [RFC PATCH v6 2/2] nvmem: Add Vybrid OCOTP and OCROM support
On 15-06-23 21:31:41, Stefan Wahren wrote: Hi Sanchayan, Sanchayan Maity maitysancha...@gmail.com hat am 23. Juni 2015 um 15:44 geschrieben: The patch adds support for the On Chip One Time Programmable Peripheral (OCOTP) and On Chip ROM (OCROM) support. On Vybrid OCOTP contain data like SoC ID, MAC address and OCROM has the revision ID. Signed-off-by: Sanchayan Maity maitysancha...@gmail.com --- drivers/nvmem/Kconfig | 11 + drivers/nvmem/Makefile | 2 ++ drivers/nvmem/vf610-ocotp.c | 60 + 3 files changed, 73 insertions(+) create mode 100644 drivers/nvmem/vf610-ocotp.c diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig index 17f1a57..557c1e0 100644 --- a/drivers/nvmem/Kconfig +++ b/drivers/nvmem/Kconfig @@ -33,4 +33,15 @@ 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 + select REGMAP_MMIO how do you come to the conclusion that Vybrid On-Chip OTP is accessable via MMIO? Frankly speaking I just changed the naming conventions and followed the qfrom and sunxi sid examples in Srinivas's patches. I just tested it without the select REGMAP_MMIO and it works just fine. - 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: [PATCH v5 06/11] nvmem: Add bindings for simple nvmem framework
Hello Srinivas,
On 15-05-21 17:44:12, Srinivas Kandagatla wrote:
> This patch adds bindings for simple nvmem framework which allows nvmem
> consumers to talk to nvmem providers to get access to nvmem cell data.
>
> Signed-off-by: Maxime Ripard
> [Maxime Ripard: intial version of eeprom framework]
> Signed-off-by: Srinivas Kandagatla
> ---
> Documentation/devicetree/bindings/nvmem/nvmem.txt | 84
> +++
> 1 file changed, 84 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/nvmem/nvmem.txt
>
> diff --git a/Documentation/devicetree/bindings/nvmem/nvmem.txt
> b/Documentation/devicetree/bindings/nvmem/nvmem.txt
> new file mode 100644
> index 000..ecea654
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/nvmem/nvmem.txt
> @@ -0,0 +1,84 @@
> += NVMEM Data Device Tree Bindings =
> +
> +This binding is intended to represent the location of hardware
> +configuration data stored in NVMEMs.
> +
> +On a significant proportion of boards, the manufacturer has stored
> +some data on NVMEM, for the OS to be able to retrieve these information
> +and act upon it. Obviously, the OS has to know about where to retrieve
> +these data from, and where they are stored on the storage device.
> +
> +This document is here to document this.
> +
> += Data providers =
> +Contains bindings specific to provider drivers and data cells as children
> +to this node.
> +
> +Optional properties:
> + read-only: Mark the provider as read only.
> +
> += Data cells =
> +These are the child nodes of the provider which contain data cell
> +information like offset and size in nvmem provider.
> +
> +Required properties:
> +reg: specifies the offset in byte within that storage device, start bit
> + in the byte and the length in bits of the data we care about.
> + There could be more then one offset-length pairs in this property.
> +
> +Optional properties:
> +
> +bit-offset: specifies the offset in bit within the address range specified
> + by reg property. Can take values from 0-7.
> +nbits: specifies number of bits this cell occupies starting from bit-offset.
> +
> +For example:
> +
> + /* Provider */
> + qfprom: qfprom@0070 {
> + ...
> +
> + /* Data cells */
> + tsens_calibration: calib@404 {
> + reg = <0x404 0x10>;
> + };
> +
> + tsens_calibration_bckp: calib_bckp@504 {
> + reg = <0x504 0x11>;
> + bit-offset = 6;
> + nbits = 128;
> + };
> +
> + pvs_version: pvs-version@6 {
> + reg = <0x6 0x2>
> + bit-offset = 7;
> + nbits = 2;
> + };
> +
> + speed_bin: speed-bin@c{
> + reg = <0xc 0x1>;
> + bit-offset = 2;
> + nbits = 3;
> +
> + };
> + ...
> + };
We have a need of exposing information like SoC ID, revision and such
which is what this nvmem framework proposes to be suitable for. Till
now I was trying a different approach for the same [1].
The On Chip One Time Programmable block on the Vybrid can be handled
nicely with this nvmem framework however I had a few queries with
regards to the framework after trying it on a Vybrid VF61 SoC.
1. From what I understand, getting the information in hex is the only
way possible as of now? Would it be possible to expose the information
as strings from different paths under /sys/class/nvmem/*/nvmem/?
For example, if I have a sub node as below
ocotp: ocotp@400a5000 {
compatible = "fsl,vf610-ocotp";
reg = <0x400a5000 0x1000>;
ocotp_cfg0: cfg0@410 {
reg = <0x410 0x1>;
};
ocotp_cfg1: cfg1@420 {
reg = <0x420 0x1>;
};
};
The values from the above register locations represented by the two
sub nodes above can perhaps be exposed as strings? Even if they are
not exposed as strings, making them available in separate paths, is
that something which can be done? So depending on the sub node as
above, /sys/class/nvmem/ocotp0/nvmem/cfg0/ would give values from
the registers specified.
Basically the output of /sys/class/nvmem/*/nvmem being restricted
to only the subnodes specified is what I was hoping to get along
with separate paths.
2. What if the required information is scattered across different memory
regions? In my case, the SoC ID is available from one OCOTP peripheral
block, the revision is in a separate ROM area at the start of the memory
map and some CPU information I am interested in is in another memory
region. I am not sure what would be the right way to approach it with
the nvmem framework.
[1] https://lkml.org/lkml/2015/6/5/189
Thanks & Regards,
Sanchayan Maity.
--
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
Re: [PATCH v5 06/11] nvmem: Add bindings for simple nvmem framework
Hello Srinivas,
On 15-05-21 17:44:12, Srinivas Kandagatla wrote:
This patch adds bindings for simple nvmem framework which allows nvmem
consumers to talk to nvmem providers to get access to nvmem cell data.
Signed-off-by: Maxime Ripard maxime.rip...@free-electrons.com
[Maxime Ripard: intial version of eeprom framework]
Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@linaro.org
---
Documentation/devicetree/bindings/nvmem/nvmem.txt | 84
+++
1 file changed, 84 insertions(+)
create mode 100644 Documentation/devicetree/bindings/nvmem/nvmem.txt
diff --git a/Documentation/devicetree/bindings/nvmem/nvmem.txt
b/Documentation/devicetree/bindings/nvmem/nvmem.txt
new file mode 100644
index 000..ecea654
--- /dev/null
+++ b/Documentation/devicetree/bindings/nvmem/nvmem.txt
@@ -0,0 +1,84 @@
+= NVMEM Data Device Tree Bindings =
+
+This binding is intended to represent the location of hardware
+configuration data stored in NVMEMs.
+
+On a significant proportion of boards, the manufacturer has stored
+some data on NVMEM, for the OS to be able to retrieve these information
+and act upon it. Obviously, the OS has to know about where to retrieve
+these data from, and where they are stored on the storage device.
+
+This document is here to document this.
+
+= Data providers =
+Contains bindings specific to provider drivers and data cells as children
+to this node.
+
+Optional properties:
+ read-only: Mark the provider as read only.
+
+= Data cells =
+These are the child nodes of the provider which contain data cell
+information like offset and size in nvmem provider.
+
+Required properties:
+reg: specifies the offset in byte within that storage device, start bit
+ in the byte and the length in bits of the data we care about.
+ There could be more then one offset-length pairs in this property.
+
+Optional properties:
+
+bit-offset: specifies the offset in bit within the address range specified
+ by reg property. Can take values from 0-7.
+nbits: specifies number of bits this cell occupies starting from bit-offset.
+
+For example:
+
+ /* Provider */
+ qfprom: qfprom@0070 {
+ ...
+
+ /* Data cells */
+ tsens_calibration: calib@404 {
+ reg = 0x404 0x10;
+ };
+
+ tsens_calibration_bckp: calib_bckp@504 {
+ reg = 0x504 0x11;
+ bit-offset = 6;
+ nbits = 128;
+ };
+
+ pvs_version: pvs-version@6 {
+ reg = 0x6 0x2
+ bit-offset = 7;
+ nbits = 2;
+ };
+
+ speed_bin: speed-bin@c{
+ reg = 0xc 0x1;
+ bit-offset = 2;
+ nbits = 3;
+
+ };
+ ...
+ };
We have a need of exposing information like SoC ID, revision and such
which is what this nvmem framework proposes to be suitable for. Till
now I was trying a different approach for the same [1].
The On Chip One Time Programmable block on the Vybrid can be handled
nicely with this nvmem framework however I had a few queries with
regards to the framework after trying it on a Vybrid VF61 SoC.
1. From what I understand, getting the information in hex is the only
way possible as of now? Would it be possible to expose the information
as strings from different paths under /sys/class/nvmem/*/nvmem/?
For example, if I have a sub node as below
ocotp: ocotp@400a5000 {
compatible = fsl,vf610-ocotp;
reg = 0x400a5000 0x1000;
ocotp_cfg0: cfg0@410 {
reg = 0x410 0x1;
};
ocotp_cfg1: cfg1@420 {
reg = 0x420 0x1;
};
};
The values from the above register locations represented by the two
sub nodes above can perhaps be exposed as strings? Even if they are
not exposed as strings, making them available in separate paths, is
that something which can be done? So depending on the sub node as
above, /sys/class/nvmem/ocotp0/nvmem/cfg0/ would give values from
the registers specified.
Basically the output of /sys/class/nvmem/*/nvmem being restricted
to only the subnodes specified is what I was hoping to get along
with separate paths.
2. What if the required information is scattered across different memory
regions? In my case, the SoC ID is available from one OCOTP peripheral
block, the revision is in a separate ROM area at the start of the memory
map and some CPU information I am interested in is in another memory
region. I am not sure what would be the right way to approach it with
the nvmem framework.
[1] https://lkml.org/lkml/2015/6/5/189
Thanks Regards,
Sanchayan Maity.
snip
--
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
Re: [PATCH v1 1/2] iio: adc: Determine sampling frequencies by using minimum sample time
Hello,
On 15-06-14 12:16:07, Jonathan Cameron wrote:
> On 08/06/15 18:51, Rob Herring wrote:
> > On Mon, Jun 8, 2015 at 12:42 AM, Sanchayan Maity
> > wrote:
> >> The driver currently does not take into account the minimum sample time
> >> as per the Figure 6-8 Chapter 9.1.1 12-bit ADC electrical characteristics.
> >> We set a static amount of cycles instead of considering the sample time as
> >> a given value, which depends on hardware characteristics.
> >>
> >> Determine sampling frequencies by first reading the device tree property
> >> node and then calculating the required Long Sample Time Adder (LSTAdder)
> >> value based on the ADC clock frequency and sample time value obtained
> >> from the device tree, this LSTAdder value is then used for calculating
> >> the sampling frequencies possible.
> >>
> >> Signed-off-by: Sanchayan Maity
> >> ---
> >> .../devicetree/bindings/iio/adc/vf610-adc.txt | 6 ++
> >> drivers/iio/adc/vf610_adc.c| 74
> >> --
> >> 2 files changed, 76 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/Documentation/devicetree/bindings/iio/adc/vf610-adc.txt
> >> b/Documentation/devicetree/bindings/iio/adc/vf610-adc.txt
> >> index 3eb40e2..4419126 100644
> >> --- a/Documentation/devicetree/bindings/iio/adc/vf610-adc.txt
> >> +++ b/Documentation/devicetree/bindings/iio/adc/vf610-adc.txt
> >> @@ -17,6 +17,11 @@ Recommended properties:
> >>- Frequency in normal mode (ADLPC=0, ADHSC=0)
> >>- Frequency in high-speed mode (ADLPC=0, ADHSC=1)
> >>- Frequency in low-power mode (ADLPC=1, ADHSC=0)
> >> + - fsl,min-sample-time: Minimum sampling time in nanoseconds. This value
> >> has
> >
> > Add a "-ns' suffix.
> >
> > Seems like this could be a common ADC property.
> Could be but so far this is a first I think...
>
> Obviously there are constraints on when you'll get a 'good' answer from any
> ADC but often they are not so well defined in the datasheets as we have here.
>
> Having said that, certainly could turn up elsewhere so I've no problem with
> having it as normal (non vendor specific) property.
Will change this to a non vendor specific property and send v2.
Thanks for the review.
Regards,
Sanchayan.
> >
> >> +to be chosen according to the conversion mode and the connected analog
> >> +source resistance (R_as) and capacitance (C_as). Refer the
> >> datasheet's
> >> +operating requirements. A safe default across a wide range of
> >> R_as and
> >> +C_as as well as conversion modes is 1000ns.
> >>
> >> Example:
> >> adc0: adc@4003b000 {
> >> @@ -27,5 +32,6 @@ adc0: adc@4003b000 {
> >> clock-names = "adc";
> >> fsl,adck-max-frequency = <3000>, <4000>,
> >> <2000>;
> >> + fsl,min-sample-time = <1000>;
> >> vref-supply = <_vcc_3v3_mcu>;
> >> };
> >> diff --git a/drivers/iio/adc/vf610_adc.c b/drivers/iio/adc/vf610_adc.c
> >> index 480f335..bcae8fd 100644
> >> --- a/drivers/iio/adc/vf610_adc.c
> >> +++ b/drivers/iio/adc/vf610_adc.c
> >> @@ -68,6 +68,9 @@
> >> #define VF610_ADC_CLK_DIV8 0x60
> >> #define VF610_ADC_CLK_MASK 0x60
> >> #define VF610_ADC_ADLSMP_LONG 0x10
> >> +#define VF610_ADC_ADSTS_SHORT 0x100
> >> +#define VF610_ADC_ADSTS_NORMAL 0x200
> >> +#define VF610_ADC_ADSTS_LONG0x300
> >> #define VF610_ADC_ADSTS_MASK 0x300
> >> #define VF610_ADC_ADLPC_EN 0x80
> >> #define VF610_ADC_ADHSC_EN 0x400
> >> @@ -124,6 +127,17 @@ enum conversion_mode_sel {
> >> VF610_ADC_CONV_LOW_POWER,
> >> };
> >>
> >> +enum lst_adder_sel {
> >> + VF610_ADCK_CYCLES_3,
> >> + VF610_ADCK_CYCLES_5,
> >> + VF610_ADCK_CYCLES_7,
> >> + VF610_ADCK_CYCLES_9,
> >> + VF610_ADCK_CYCLES_13,
> >> + VF610_ADCK_CYCLES_17,
> >> + VF610_ADCK_CYCLES_21,
> >> + VF610_ADCK_CYCLES_25,
> >> +};
> >> +
> >> struct vf610_adc_feature {
> >> enum clk_selclk_sel;
> >> enum vol_refvol_ref;
> >> @@ -132,6 +146,8 @@ struct vf610_adc_feature {
> >> int clk_div;
> >> int sample_rate;
> >> int res_mode;
> >> + u32 lst_adder_index;
> >> + u32 default_sample_time;
> >>
> >> boolcalibration;
> >> boolovwren;
> >> @@ -155,11 +171,13 @@ struct vf610_adc {
> >> };
> >>
> >> static const u32 vf610_hw_avgs[] = { 1, 4, 8, 16, 32 };
> >> +static const u32 vf610_lst_adder[] = { 3, 5, 7, 9, 13, 17, 21, 25 };
> >>
> >> static inline void vf610_adc_calculate_rates(struct vf610_adc *info)
> >> {
> >> struct vf610_adc_feature *adc_feature = >adc_feature;
> >> unsigned long adck_rate, ipg_rate = clk_get_rate(info->clk);
> >> + u32 adck_period, lst_addr_min;
> >> int divisor, i;
> >>
> >> adck_rate = info->max_adck_rate[adc_feature->conv_mode];
> >> @@ -174,6 +192,19 @@ static inline void
Re: [PATCH v1 1/2] iio: adc: Determine sampling frequencies by using minimum sample time
Hello,
On 15-06-14 12:16:07, Jonathan Cameron wrote:
On 08/06/15 18:51, Rob Herring wrote:
On Mon, Jun 8, 2015 at 12:42 AM, Sanchayan Maity
maitysancha...@gmail.com wrote:
The driver currently does not take into account the minimum sample time
as per the Figure 6-8 Chapter 9.1.1 12-bit ADC electrical characteristics.
We set a static amount of cycles instead of considering the sample time as
a given value, which depends on hardware characteristics.
Determine sampling frequencies by first reading the device tree property
node and then calculating the required Long Sample Time Adder (LSTAdder)
value based on the ADC clock frequency and sample time value obtained
from the device tree, this LSTAdder value is then used for calculating
the sampling frequencies possible.
Signed-off-by: Sanchayan Maity maitysancha...@gmail.com
---
.../devicetree/bindings/iio/adc/vf610-adc.txt | 6 ++
drivers/iio/adc/vf610_adc.c| 74
--
2 files changed, 76 insertions(+), 4 deletions(-)
diff --git a/Documentation/devicetree/bindings/iio/adc/vf610-adc.txt
b/Documentation/devicetree/bindings/iio/adc/vf610-adc.txt
index 3eb40e2..4419126 100644
--- a/Documentation/devicetree/bindings/iio/adc/vf610-adc.txt
+++ b/Documentation/devicetree/bindings/iio/adc/vf610-adc.txt
@@ -17,6 +17,11 @@ Recommended properties:
- Frequency in normal mode (ADLPC=0, ADHSC=0)
- Frequency in high-speed mode (ADLPC=0, ADHSC=1)
- Frequency in low-power mode (ADLPC=1, ADHSC=0)
+ - fsl,min-sample-time: Minimum sampling time in nanoseconds. This value
has
Add a -ns' suffix.
Seems like this could be a common ADC property.
Could be but so far this is a first I think...
Obviously there are constraints on when you'll get a 'good' answer from any
ADC but often they are not so well defined in the datasheets as we have here.
Having said that, certainly could turn up elsewhere so I've no problem with
having it as normal (non vendor specific) property.
Will change this to a non vendor specific property and send v2.
Thanks for the review.
Regards,
Sanchayan.
+to be chosen according to the conversion mode and the connected analog
+source resistance (R_as) and capacitance (C_as). Refer the
datasheet's
+operating requirements. A safe default across a wide range of
R_as and
+C_as as well as conversion modes is 1000ns.
Example:
adc0: adc@4003b000 {
@@ -27,5 +32,6 @@ adc0: adc@4003b000 {
clock-names = adc;
fsl,adck-max-frequency = 3000, 4000,
2000;
+ fsl,min-sample-time = 1000;
vref-supply = reg_vcc_3v3_mcu;
};
diff --git a/drivers/iio/adc/vf610_adc.c b/drivers/iio/adc/vf610_adc.c
index 480f335..bcae8fd 100644
--- a/drivers/iio/adc/vf610_adc.c
+++ b/drivers/iio/adc/vf610_adc.c
@@ -68,6 +68,9 @@
#define VF610_ADC_CLK_DIV8 0x60
#define VF610_ADC_CLK_MASK 0x60
#define VF610_ADC_ADLSMP_LONG 0x10
+#define VF610_ADC_ADSTS_SHORT 0x100
+#define VF610_ADC_ADSTS_NORMAL 0x200
+#define VF610_ADC_ADSTS_LONG0x300
#define VF610_ADC_ADSTS_MASK 0x300
#define VF610_ADC_ADLPC_EN 0x80
#define VF610_ADC_ADHSC_EN 0x400
@@ -124,6 +127,17 @@ enum conversion_mode_sel {
VF610_ADC_CONV_LOW_POWER,
};
+enum lst_adder_sel {
+ VF610_ADCK_CYCLES_3,
+ VF610_ADCK_CYCLES_5,
+ VF610_ADCK_CYCLES_7,
+ VF610_ADCK_CYCLES_9,
+ VF610_ADCK_CYCLES_13,
+ VF610_ADCK_CYCLES_17,
+ VF610_ADCK_CYCLES_21,
+ VF610_ADCK_CYCLES_25,
+};
+
struct vf610_adc_feature {
enum clk_selclk_sel;
enum vol_refvol_ref;
@@ -132,6 +146,8 @@ struct vf610_adc_feature {
int clk_div;
int sample_rate;
int res_mode;
+ u32 lst_adder_index;
+ u32 default_sample_time;
boolcalibration;
boolovwren;
@@ -155,11 +171,13 @@ struct vf610_adc {
};
static const u32 vf610_hw_avgs[] = { 1, 4, 8, 16, 32 };
+static const u32 vf610_lst_adder[] = { 3, 5, 7, 9, 13, 17, 21, 25 };
static inline void vf610_adc_calculate_rates(struct vf610_adc *info)
{
struct vf610_adc_feature *adc_feature = info-adc_feature;
unsigned long adck_rate, ipg_rate = clk_get_rate(info-clk);
+ u32 adck_period, lst_addr_min;
int divisor, i;
adck_rate = info-max_adck_rate[adc_feature-conv_mode];
@@ -174,6 +192,19 @@ static inline void vf610_adc_calculate_rates(struct
vf610_adc *info)
}
/*
+* Determine the long sample time adder value to be used based
+* on the default minimum sample time provided.
+*/
+ adck_period = NSEC_PER_SEC / adck_rate;
+ lst_addr_min =
Re: [PATCH v5 2/2] soc: Add driver for Freescale Vybrid Platform
Hello Paul,
On 15-06-06 12:26:07, Paul Bolle wrote:
> On Fri, 2015-06-05 at 14:52 +0530, Sanchayan Maity wrote:
> > --- /dev/null
> > +++ b/drivers/soc/fsl/Kconfig
>
> > +config SOC_BUS_VF610
> > + tristate "SoC bus device for the Freescale Vybrid platform"
> > + depends on SOC_VF610
> > + select SOC_BUS
> > + help
> > +Include support for the SoC bus on the Freescale Vybrid platform
> > +providing some sysfs information about the module variant.
>
> > --- /dev/null
> > +++ b/drivers/soc/fsl/Makefile
>
> > +obj-$(CONFIG_SOC_BUS_VF610)+= soc-vf610.o
>
> > --- /dev/null
> > +++ b/drivers/soc/fsl/soc-vf610.c
> > @@ -0,0 +1,166 @@
> > +/*
> > + * Copyright 2015 Toradex AG
> > + *
> > + * Author: 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, as
> > + * published by the Free Software Foundation.
> > + *
> > + */
> > +
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +#define DRIVER_NAME "vf610-soc-bus"
> > +
> > +#define MSCM_CPxCOUNT_OFFSET 0x002C
> > +#define MSCM_CPxCFG1_OFFSET0x0014
> > +
> > +struct vf610_soc {
> > + struct device *dev;
> > + struct soc_device_attribute *soc_dev_attr;
> > + struct soc_device *soc_dev;
> > +};
> > +
> > +static int vf610_soc_probe(struct platform_device *pdev)
> > +{
> > + struct vf610_soc *info;
> > + struct regmap *ocotp_regmap, *mscm_regmap, *rom_regmap;
> > + struct device *dev = >dev;
> > + struct device_node *node = pdev->dev.of_node;
> > + struct device_node *soc_node;
> > + struct of_phandle_args pargs;
> > + char soc_type[] = "xx0";
> > + u32 cfg0_offset, cfg1_offset, rom_rev_offset;
> > + u32 soc_id1, soc_id2, rom_rev;
> > + u32 cpxcount, cpxcfg1;
> > + u64 soc_id;
> > + int ret;
> > +
> > + info = devm_kzalloc(>dev, sizeof(struct vf610_soc), GFP_KERNEL);
> > + if (!info)
> > + return -ENOMEM;
> > +
> > + info->dev = >dev;
> > + platform_set_drvdata(pdev, info);
> > +
> > + mscm_regmap = syscon_node_to_regmap(node);
> > + if (IS_ERR(mscm_regmap)) {
> > + dev_err(dev, "regmap lookup for mscm failed\n");
> > + return PTR_ERR(mscm_regmap);
> > + }
> > +
> > + soc_node = of_find_node_by_path("/soc");
> > +
> > + ret = of_parse_phandle_with_fixed_args(soc_node,
> > + "ocotp-cfg", 2, 0, );
> > + if (ret) {
> > + dev_err(dev, "lookup failed for ocotp-cfg node %d\n", ret);
> > + return ret;
> > + }
> > +
> > + ocotp_regmap = syscon_node_to_regmap(pargs.np);
> > + if (IS_ERR(ocotp_regmap)) {
> > + of_node_put(pargs.np);
> > + dev_err(dev, "regmap lookup for ocotp failed\n");
> > + return PTR_ERR(ocotp_regmap);
> > + }
> > +
> > + cfg0_offset = pargs.args[0];
> > + cfg1_offset = pargs.args[1];
> > + of_node_put(pargs.np);
> > +
> > + ret = of_parse_phandle_with_fixed_args(soc_node,
> > + "rom-revision", 1, 0, );
> > + if (ret) {
> > + dev_err(dev, "lookup failed for rom-revision node %d\n", ret);
> > + return ret;
> > + }
> > +
> > + rom_regmap = syscon_node_to_regmap(pargs.np);
> > + if (IS_ERR(rom_regmap)) {
> > + of_node_put(pargs.np);
> > + dev_err(dev, "regmap lookup for ocrom failed\n");
> > + return PTR_ERR(rom_regmap);
> > + }
> > +
> > + rom_rev_offset = pargs.args[0];
> > + of_node_put(pargs.np);
> > +
> > + ret = regmap_read(ocotp_regmap, cfg0_offset, _id1);
> > + if (ret)
> > + return -ENODEV;
> > +
> > + ret = regmap_read(ocotp_regmap, cfg1_offset, _id2);
> > + if (ret)
> > + return -ENODEV;
> > +
> > + soc_id = (u64) soc_id1 << 32 | soc_id2;
> > + add_device_randomness(_id, sizeof(soc_id));
> > +
> > + ret = regmap_read(mscm_regmap, MSCM_CPxCOUNT_OFFSET, );
> > + if (ret)
> > + return -ENODEV;
> > +
> > + ret = regmap_read(mscm_regmap, MSCM_CPxCFG1_OFFSET, );
> > + if (ret)
> > + return -ENODEV;
> > +
> > + soc_type[0] = cpxcount ? '6' : '5'; /* Dual Core => VF6x0 */
> > + soc_type[1] = cpxcfg1 ? '1' : '0'; /* L2 Cache => VFx10 */
> > +
> > + ret = regmap_read(rom_regmap, rom_rev_offset, _rev);
> > + if (ret)
> > + return -ENODEV;
> > +
> > + info->soc_dev_attr = devm_kzalloc(>dev,
> > + sizeof(info->soc_dev_attr), GFP_KERNEL);
> > + if (!info->soc_dev_attr)
> > + return -ENOMEM;
> > +
> > + info->soc_dev_attr->machine = devm_kasprintf(>dev,
> > + GFP_KERNEL, "Freescale Vybrid");
> > + info->soc_dev_attr->soc_id = devm_kasprintf(>dev,
> > + GFP_KERNEL, "%016llx", soc_id);
> > + info->soc_dev_attr->family = devm_kasprintf(>dev,
> > +
Re: [PATCH v5 1/2] ARM: dts: vfxxx: Add OCOTP and OCROM nodes
Hello Maxime,
On 15-06-06 13:58:33, Maxime Ripard wrote:
> Hi,
>
> On Fri, Jun 05, 2015 at 02:52:37PM +0530, Sanchayan Maity wrote:
> > Add a device tree node for the On-Chip One Time Programmable
> > Controller (OCOTP) and the On-Chip ROM.
> >
> > Signed-off-by: Sanchayan Maity
> > ---
> > arch/arm/boot/dts/vfxxx.dtsi | 12
> > 1 file changed, 12 insertions(+)
> >
> > diff --git a/arch/arm/boot/dts/vfxxx.dtsi b/arch/arm/boot/dts/vfxxx.dtsi
> > index 2f4b04d..ed9b6b8 100644
> > --- a/arch/arm/boot/dts/vfxxx.dtsi
> > +++ b/arch/arm/boot/dts/vfxxx.dtsi
> > @@ -56,6 +56,13 @@
> > compatible = "simple-bus";
> > interrupt-parent = <_ir>;
> > ranges;
> > + ocotp-cfg = < 0x410 0x420>;
> > + rom-revision = < 0x80>;
> > +
> > + ocrom: ocrom {
> > + compatible = "fsl,vf610-ocrom", "syscon";
> > + reg = <0x 0x18000>;
> > + };
>
> Shouldn't that use the new nvmem framework currently being discussed,
> instead of defining custom bindings?
I am aware of those nvmem framework patches by Srinivas, however from what
I can see that framework has not been merged yet atleast from the mail
chain that I see for v5 of the patchset so I have not properly looked into
that. Mark Brown did apply the regmap patches to his tree, but has the rest
of the framework been merged or is it expected to be merged?
If it is expected to be merged and if that framework is a fit for what we
are trying to do here and it seems so on a quick glance, then I can go with
that. However I still did like to hear from Arnd or have some more comments
before I go for that approach with the next respin.
- 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: [PATCH v5 2/2] soc: Add driver for Freescale Vybrid Platform
Hello Paul,
On 15-06-06 12:26:07, Paul Bolle wrote:
On Fri, 2015-06-05 at 14:52 +0530, Sanchayan Maity wrote:
--- /dev/null
+++ b/drivers/soc/fsl/Kconfig
+config SOC_BUS_VF610
+ tristate SoC bus device for the Freescale Vybrid platform
+ depends on SOC_VF610
+ select SOC_BUS
+ help
+Include support for the SoC bus on the Freescale Vybrid platform
+providing some sysfs information about the module variant.
--- /dev/null
+++ b/drivers/soc/fsl/Makefile
+obj-$(CONFIG_SOC_BUS_VF610)+= soc-vf610.o
--- /dev/null
+++ b/drivers/soc/fsl/soc-vf610.c
@@ -0,0 +1,166 @@
+/*
+ * Copyright 2015 Toradex AG
+ *
+ * Author: 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, as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#include linux/mfd/syscon.h
+#include linux/of_platform.h
+#include linux/regmap.h
+#include linux/random.h
+#include linux/slab.h
+#include linux/sys_soc.h
+
+#define DRIVER_NAME vf610-soc-bus
+
+#define MSCM_CPxCOUNT_OFFSET 0x002C
+#define MSCM_CPxCFG1_OFFSET0x0014
+
+struct vf610_soc {
+ struct device *dev;
+ struct soc_device_attribute *soc_dev_attr;
+ struct soc_device *soc_dev;
+};
+
+static int vf610_soc_probe(struct platform_device *pdev)
+{
+ struct vf610_soc *info;
+ struct regmap *ocotp_regmap, *mscm_regmap, *rom_regmap;
+ struct device *dev = pdev-dev;
+ struct device_node *node = pdev-dev.of_node;
+ struct device_node *soc_node;
+ struct of_phandle_args pargs;
+ char soc_type[] = xx0;
+ u32 cfg0_offset, cfg1_offset, rom_rev_offset;
+ u32 soc_id1, soc_id2, rom_rev;
+ u32 cpxcount, cpxcfg1;
+ u64 soc_id;
+ int ret;
+
+ info = devm_kzalloc(pdev-dev, sizeof(struct vf610_soc), GFP_KERNEL);
+ if (!info)
+ return -ENOMEM;
+
+ info-dev = pdev-dev;
+ platform_set_drvdata(pdev, info);
+
+ mscm_regmap = syscon_node_to_regmap(node);
+ if (IS_ERR(mscm_regmap)) {
+ dev_err(dev, regmap lookup for mscm failed\n);
+ return PTR_ERR(mscm_regmap);
+ }
+
+ soc_node = of_find_node_by_path(/soc);
+
+ ret = of_parse_phandle_with_fixed_args(soc_node,
+ ocotp-cfg, 2, 0, pargs);
+ if (ret) {
+ dev_err(dev, lookup failed for ocotp-cfg node %d\n, ret);
+ return ret;
+ }
+
+ ocotp_regmap = syscon_node_to_regmap(pargs.np);
+ if (IS_ERR(ocotp_regmap)) {
+ of_node_put(pargs.np);
+ dev_err(dev, regmap lookup for ocotp failed\n);
+ return PTR_ERR(ocotp_regmap);
+ }
+
+ cfg0_offset = pargs.args[0];
+ cfg1_offset = pargs.args[1];
+ of_node_put(pargs.np);
+
+ ret = of_parse_phandle_with_fixed_args(soc_node,
+ rom-revision, 1, 0, pargs);
+ if (ret) {
+ dev_err(dev, lookup failed for rom-revision node %d\n, ret);
+ return ret;
+ }
+
+ rom_regmap = syscon_node_to_regmap(pargs.np);
+ if (IS_ERR(rom_regmap)) {
+ of_node_put(pargs.np);
+ dev_err(dev, regmap lookup for ocrom failed\n);
+ return PTR_ERR(rom_regmap);
+ }
+
+ rom_rev_offset = pargs.args[0];
+ of_node_put(pargs.np);
+
+ ret = regmap_read(ocotp_regmap, cfg0_offset, soc_id1);
+ if (ret)
+ return -ENODEV;
+
+ ret = regmap_read(ocotp_regmap, cfg1_offset, soc_id2);
+ if (ret)
+ return -ENODEV;
+
+ soc_id = (u64) soc_id1 32 | soc_id2;
+ add_device_randomness(soc_id, sizeof(soc_id));
+
+ ret = regmap_read(mscm_regmap, MSCM_CPxCOUNT_OFFSET, cpxcount);
+ if (ret)
+ return -ENODEV;
+
+ ret = regmap_read(mscm_regmap, MSCM_CPxCFG1_OFFSET, cpxcfg1);
+ if (ret)
+ return -ENODEV;
+
+ soc_type[0] = cpxcount ? '6' : '5'; /* Dual Core = VF6x0 */
+ soc_type[1] = cpxcfg1 ? '1' : '0'; /* L2 Cache = VFx10 */
+
+ ret = regmap_read(rom_regmap, rom_rev_offset, rom_rev);
+ if (ret)
+ return -ENODEV;
+
+ info-soc_dev_attr = devm_kzalloc(pdev-dev,
+ sizeof(info-soc_dev_attr), GFP_KERNEL);
+ if (!info-soc_dev_attr)
+ return -ENOMEM;
+
+ info-soc_dev_attr-machine = devm_kasprintf(pdev-dev,
+ GFP_KERNEL, Freescale Vybrid);
+ info-soc_dev_attr-soc_id = devm_kasprintf(pdev-dev,
+ GFP_KERNEL, %016llx, soc_id);
+ info-soc_dev_attr-family = devm_kasprintf(pdev-dev,
+ GFP_KERNEL, Freescale Vybrid VF%s,
+ soc_type);
+ info-soc_dev_attr-revision = devm_kasprintf(pdev-dev,
+
Re: [PATCH v5 1/2] ARM: dts: vfxxx: Add OCOTP and OCROM nodes
Hello Maxime,
On 15-06-06 13:58:33, Maxime Ripard wrote:
Hi,
On Fri, Jun 05, 2015 at 02:52:37PM +0530, Sanchayan Maity wrote:
Add a device tree node for the On-Chip One Time Programmable
Controller (OCOTP) and the On-Chip ROM.
Signed-off-by: Sanchayan Maity maitysancha...@gmail.com
---
arch/arm/boot/dts/vfxxx.dtsi | 12
1 file changed, 12 insertions(+)
diff --git a/arch/arm/boot/dts/vfxxx.dtsi b/arch/arm/boot/dts/vfxxx.dtsi
index 2f4b04d..ed9b6b8 100644
--- a/arch/arm/boot/dts/vfxxx.dtsi
+++ b/arch/arm/boot/dts/vfxxx.dtsi
@@ -56,6 +56,13 @@
compatible = simple-bus;
interrupt-parent = mscm_ir;
ranges;
+ ocotp-cfg = ocotp 0x410 0x420;
+ rom-revision = ocrom 0x80;
+
+ ocrom: ocrom {
+ compatible = fsl,vf610-ocrom, syscon;
+ reg = 0x 0x18000;
+ };
Shouldn't that use the new nvmem framework currently being discussed,
instead of defining custom bindings?
I am aware of those nvmem framework patches by Srinivas, however from what
I can see that framework has not been merged yet atleast from the mail
chain that I see for v5 of the patchset so I have not properly looked into
that. Mark Brown did apply the regmap patches to his tree, but has the rest
of the framework been merged or is it expected to be merged?
If it is expected to be merged and if that framework is a fit for what we
are trying to do here and it seems so on a quick glance, then I can go with
that. However I still did like to hear from Arnd or have some more comments
before I go for that approach with the next respin.
- 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: [PATCH v4 2/2] soc: Add driver for Freescale Vybrid Platform
Hello,
On 15-05-27 09:31:50, Paul Bolle wrote:
> On Tue, 2015-05-26 at 17:06 +0530, Sanchayan Maity wrote:
> > --- /dev/null
> > +++ b/drivers/soc/fsl/Kconfig
>
> > +config SOC_VF610
> > + bool "SoC bus device for the Freescale Vybrid platform"
> > + select SOC_BUS
> > + help
> > +Include support for the SoC bus on the Freescale Vybrid platform
> > +providing some sysfs information about the module variant.
> > \ No newline at end of file
>
> (That review comment is courtesy of git.)
Will fix it.
>
> > --- /dev/null
> > +++ b/drivers/soc/fsl/Makefile
>
> > +obj-$(CONFIG_SOC_VF610)+= soc-vf610.o
>
> > --- /dev/null
> > +++ b/drivers/soc/fsl/soc-vf610.c
>
> > +MODULE_DEVICE_TABLE(of, vf610_soc_bus_match);
>
> > +module_platform_driver(vf610_soc_driver);
>
> (The series starting at https://lkml.org/lkml/2015/5/10/131 would allow
> to use builtin_platform_driver() for built-in only code.)
Thanks for bringing this to my attention. I am subscribed to the mailing
list however this skipped me.
>
> > +MODULE_DESCRIPTION("Freescale VF610 SoC bus driver");
> > +MODULE_LICENSE("GPL v2");
>
> I think soc-vf610.o can only be built-in. But its code contains a few
> module specific macros. Was it perhaps intended for SOC_VF610 to be
> tristate?
I too think that should be built-in. Did not have an intention of making
it tristate, however while using other drivers as references, the perhaps
unneccessary stuff crept in.
The MODULE_* references can be removed along with the corresponding header
file. However that series has not been merged yet, so I can't use builtin_*
yet?
@Arnd
Are you ok with the patch in general? I can take care of the above changes
and send a new version. And once the builtin_driver stuff gets merged, I
can send a minor patch to change this module one to builtin?
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: [PATCH v4 2/2] soc: Add driver for Freescale Vybrid Platform
Hello, On 15-05-27 09:31:50, Paul Bolle wrote: On Tue, 2015-05-26 at 17:06 +0530, Sanchayan Maity wrote: --- /dev/null +++ b/drivers/soc/fsl/Kconfig +config SOC_VF610 + bool SoC bus device for the Freescale Vybrid platform + select SOC_BUS + help +Include support for the SoC bus on the Freescale Vybrid platform +providing some sysfs information about the module variant. \ No newline at end of file (That review comment is courtesy of git.) Will fix it. --- /dev/null +++ b/drivers/soc/fsl/Makefile +obj-$(CONFIG_SOC_VF610)+= soc-vf610.o --- /dev/null +++ b/drivers/soc/fsl/soc-vf610.c +MODULE_DEVICE_TABLE(of, vf610_soc_bus_match); +module_platform_driver(vf610_soc_driver); (The series starting at https://lkml.org/lkml/2015/5/10/131 would allow to use builtin_platform_driver() for built-in only code.) Thanks for bringing this to my attention. I am subscribed to the mailing list however this skipped me. +MODULE_DESCRIPTION(Freescale VF610 SoC bus driver); +MODULE_LICENSE(GPL v2); I think soc-vf610.o can only be built-in. But its code contains a few module specific macros. Was it perhaps intended for SOC_VF610 to be tristate? I too think that should be built-in. Did not have an intention of making it tristate, however while using other drivers as references, the perhaps unneccessary stuff crept in. The MODULE_* references can be removed along with the corresponding header file. However that series has not been merged yet, so I can't use builtin_* yet? @Arnd Are you ok with the patch in general? I can take care of the above changes and send a new version. And once the builtin_driver stuff gets merged, I can send a minor patch to change this module one to builtin? 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: [PATCH v3 2/2] soc: Add driver for Freescale Vybrid Platform
Hello Arnd, On 15-05-22 15:20:00, Arnd Bergmann wrote: > On Friday 22 May 2015 14:02:52 Stefan Agner wrote: > > > Can you use syscon_regmap_lookup_by_phandle instead, and put the > > > phandles in the fsl,vf610-mscm-cpucfg node? > > > > Hm, with that we would wire up hardware modules which does nothing has > > to do with each other. We just happen to need a driver which collects > > information accross the SoC. I'm not sure we should put the modules > > required into the device tree. > > > > I don't think its nice to have the compatible strings in the source > > code, however it feels more appropriate than in the device tree, IMHO... > > I see. Another option would be to point directly to the registers > you need: > > ocotp-cfg0 = < 0x10>; > ocotp-cfg1 = < 0x20>; > rom-revision = < 0x80>; > > We don't yet have an abstraction to access a register from a syscon > reference like this, but you could either roll your own here, or > add a generic abstraction. Can you tell me a little about how can I start implementing it? I am not clear on how to approach this. > > > > Also, I'd argue that the mscm should not be a syscon device at all, > > > but instead I'd use platform_get_resource()/devm_ioremap_resource() > > > to get an __iomem pointer. > > > > We need to have mscm-cpucfg to be syscon because we need to get the CPU > > personality in the MSCM interrupt router driver (irq-vf610-mscm-ir.c). > > It can be both at the same time now. > > Arnd 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: [PATCH v3 2/2] soc: Add driver for Freescale Vybrid Platform
Hello Arnd, On 15-05-22 15:20:00, Arnd Bergmann wrote: On Friday 22 May 2015 14:02:52 Stefan Agner wrote: Can you use syscon_regmap_lookup_by_phandle instead, and put the phandles in the fsl,vf610-mscm-cpucfg node? Hm, with that we would wire up hardware modules which does nothing has to do with each other. We just happen to need a driver which collects information accross the SoC. I'm not sure we should put the modules required into the device tree. I don't think its nice to have the compatible strings in the source code, however it feels more appropriate than in the device tree, IMHO... I see. Another option would be to point directly to the registers you need: ocotp-cfg0 = ocotp 0x10; ocotp-cfg1 = ocotp 0x20; rom-revision = rom 0x80; We don't yet have an abstraction to access a register from a syscon reference like this, but you could either roll your own here, or add a generic abstraction. Can you tell me a little about how can I start implementing it? I am not clear on how to approach this. Also, I'd argue that the mscm should not be a syscon device at all, but instead I'd use platform_get_resource()/devm_ioremap_resource() to get an __iomem pointer. We need to have mscm-cpucfg to be syscon because we need to get the CPU personality in the MSCM interrupt router driver (irq-vf610-mscm-ir.c). It can be both at the same time now. Arnd 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: [PATCH v3 2/2] soc: Add driver for Freescale Vybrid Platform
Hello Arnd,
On 15-05-22 13:11:46, Arnd Bergmann wrote:
> On Friday 22 May 2015 16:21:54 Sanchayan Maity wrote:
> > +#define OCOTP_CFG0_OFFSET 0x0410
> > +#define OCOTP_CFG1_OFFSET 0x0420
> > +#define MSCM_CPxCOUNT_OFFSET 0x002C
> > +#define MSCM_CPxCFG1_OFFSET0x0014
> > +#define ROM_REVISION_OFFSET0x0080
> > +
> > +static const struct of_device_id vf610_soc_bus_match[] = {
> > + { .compatible = "fsl,vf610-mscm-cpucfg", },
> > + { /* sentinel */ }
> > +};
> > +
> > +static int __init vf610_soc_init(void)
> > +{
> > + struct regmap *ocotp_regmap, *mscm_regmap, *rom_regmap;
> > + struct soc_device_attribute *soc_dev_attr;
> > + struct soc_device *soc_dev;
> > + struct device_node *np;
> > + char soc_type[] = "xx0";
> > + u32 cpxcount, cpxcfg1;
> > + u32 soc_id1, soc_id2, rom_rev;
> > + u64 soc_id;
> > + int ret;
> > +
> > + np = of_find_matching_node(NULL, vf610_soc_bus_match);
> > + if (!np)
> > + return -ENODEV;
> > +
>
> Why not use module_platform_driver() and make this a probe function instead?
Had done that but after having a look at the existing integrator and versatile
platform implementation, I changed it. Will switch to using that.
>
> > + ocotp_regmap = syscon_regmap_lookup_by_compatible("fsl,vf610-ocotp");
> > + if (IS_ERR(ocotp_regmap)) {
> > + pr_err("regmap lookup for octop failed\n");
> > + return PTR_ERR(ocotp_regmap);
> > + }
> > +
> > + mscm_regmap =
> > syscon_regmap_lookup_by_compatible("fsl,vf610-mscm-cpucfg");
> > + if (IS_ERR(mscm_regmap)) {
> > + pr_err("regmap lookup for mscm failed");
> > + return PTR_ERR(mscm_regmap);
> > + }
> > +
> > + rom_regmap = syscon_regmap_lookup_by_compatible("fsl,vf610-ocrom");
> > + if (IS_ERR(rom_regmap)) {
> > + pr_err("regmap lookup for ocrom failed");
> > + return PTR_ERR(rom_regmap);
> > + }
>
> Can you use syscon_regmap_lookup_by_phandle instead, and put the
> phandles in the fsl,vf610-mscm-cpucfg node?
Ok. Will do so.
- Sanchayan.
>
> Also, I'd argue that the mscm should not be a syscon device at all,
> but instead I'd use platform_get_resource()/devm_ioremap_resource()
> to get an __iomem pointer.
>
> Arnd
--
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: [PATCH v3 2/2] soc: Add driver for Freescale Vybrid Platform
Hello Arnd,
On 15-05-22 13:11:46, Arnd Bergmann wrote:
On Friday 22 May 2015 16:21:54 Sanchayan Maity wrote:
+#define OCOTP_CFG0_OFFSET 0x0410
+#define OCOTP_CFG1_OFFSET 0x0420
+#define MSCM_CPxCOUNT_OFFSET 0x002C
+#define MSCM_CPxCFG1_OFFSET0x0014
+#define ROM_REVISION_OFFSET0x0080
+
+static const struct of_device_id vf610_soc_bus_match[] = {
+ { .compatible = fsl,vf610-mscm-cpucfg, },
+ { /* sentinel */ }
+};
+
+static int __init vf610_soc_init(void)
+{
+ struct regmap *ocotp_regmap, *mscm_regmap, *rom_regmap;
+ struct soc_device_attribute *soc_dev_attr;
+ struct soc_device *soc_dev;
+ struct device_node *np;
+ char soc_type[] = xx0;
+ u32 cpxcount, cpxcfg1;
+ u32 soc_id1, soc_id2, rom_rev;
+ u64 soc_id;
+ int ret;
+
+ np = of_find_matching_node(NULL, vf610_soc_bus_match);
+ if (!np)
+ return -ENODEV;
+
Why not use module_platform_driver() and make this a probe function instead?
Had done that but after having a look at the existing integrator and versatile
platform implementation, I changed it. Will switch to using that.
+ ocotp_regmap = syscon_regmap_lookup_by_compatible(fsl,vf610-ocotp);
+ if (IS_ERR(ocotp_regmap)) {
+ pr_err(regmap lookup for octop failed\n);
+ return PTR_ERR(ocotp_regmap);
+ }
+
+ mscm_regmap =
syscon_regmap_lookup_by_compatible(fsl,vf610-mscm-cpucfg);
+ if (IS_ERR(mscm_regmap)) {
+ pr_err(regmap lookup for mscm failed);
+ return PTR_ERR(mscm_regmap);
+ }
+
+ rom_regmap = syscon_regmap_lookup_by_compatible(fsl,vf610-ocrom);
+ if (IS_ERR(rom_regmap)) {
+ pr_err(regmap lookup for ocrom failed);
+ return PTR_ERR(rom_regmap);
+ }
Can you use syscon_regmap_lookup_by_phandle instead, and put the
phandles in the fsl,vf610-mscm-cpucfg node?
Ok. Will do so.
- Sanchayan.
Also, I'd argue that the mscm should not be a syscon device at all,
but instead I'd use platform_get_resource()/devm_ioremap_resource()
to get an __iomem pointer.
Arnd
--
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: [PATCH v1 2/2] ARM: vf610: Add SoC bus support for Vybrid
Hello Shawn, On 15-05-19 14:24:13, Shawn Guo wrote: > On Mon, May 18, 2015 at 11:50:07AM +0530, Sanchayan Maity wrote: > > Implements SoC bus support to export SoC specific information. Read > > the unique SoC ID from the Vybrid On Chip One Time Programmable > > (OCOTP) controller, SoC specific information from the Miscellaneous > > System Control Module (MSCM), revision from the ROM revision register > > and expose it via the SoC bus infrastructure. > > > > Sample Output: > > > > root@vf:/sys/devices/soc0# cat soc_id > > df63c12a2e2161d4 > > root@vf:/sys/devices/soc0# cat family > > Freescale Vybrid VF500 > > root@vf:/sys/devices/soc0# cat revision > > 0013 > > root@vf:/sys/devices/soc0# cat machine > > Freescale Vybrid > > > > Signed-off-by: Sanchayan Maity > > --- > > arch/arm/mach-imx/mach-vf610.c | 81 > > ++ > > 1 file changed, 81 insertions(+) > > > > diff --git a/arch/arm/mach-imx/mach-vf610.c b/arch/arm/mach-imx/mach-vf610.c > > index 2e7c75b..64c78e4 100644 > > --- a/arch/arm/mach-imx/mach-vf610.c > > +++ b/arch/arm/mach-imx/mach-vf610.c > > @@ -11,6 +11,86 @@ > > #include > > #include > > #include > > +#include > > +#include > > +#include > > +#include > > +#include > > Please group headers together and sort alphabetically. Ok. Will fix this and send out a v2. The rest is acceptable? > > Shawn Thanks & Regards, Sanchayan Maity. -- 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: [PATCH v1 2/2] ARM: vf610: Add SoC bus support for Vybrid
Hello Shawn, On 15-05-19 14:24:13, Shawn Guo wrote: On Mon, May 18, 2015 at 11:50:07AM +0530, Sanchayan Maity wrote: Implements SoC bus support to export SoC specific information. Read the unique SoC ID from the Vybrid On Chip One Time Programmable (OCOTP) controller, SoC specific information from the Miscellaneous System Control Module (MSCM), revision from the ROM revision register and expose it via the SoC bus infrastructure. Sample Output: root@vf:/sys/devices/soc0# cat soc_id df63c12a2e2161d4 root@vf:/sys/devices/soc0# cat family Freescale Vybrid VF500 root@vf:/sys/devices/soc0# cat revision 0013 root@vf:/sys/devices/soc0# cat machine Freescale Vybrid Signed-off-by: Sanchayan Maity maitysancha...@gmail.com --- arch/arm/mach-imx/mach-vf610.c | 81 ++ 1 file changed, 81 insertions(+) diff --git a/arch/arm/mach-imx/mach-vf610.c b/arch/arm/mach-imx/mach-vf610.c index 2e7c75b..64c78e4 100644 --- a/arch/arm/mach-imx/mach-vf610.c +++ b/arch/arm/mach-imx/mach-vf610.c @@ -11,6 +11,86 @@ #include linux/irqchip.h #include asm/mach/arch.h #include asm/hardware/cache-l2x0.h +#include linux/slab.h +#include linux/sys_soc.h +#include linux/mfd/syscon.h +#include linux/regmap.h +#include linux/random.h Please group linux/* headers together and sort alphabetically. Ok. Will fix this and send out a v2. The rest is acceptable? Shawn Thanks Regards, Sanchayan Maity. -- 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 0/2] Implement SoC bus support for Vybrid
Hello, Ping? Any inputs? On 15-05-11 10:41:37, Sanchayan Maity wrote: > Hello, > > Currently this patchset is based of on our local branch but would like > some comments before I push this to mainline through Shawn's tree. > > This patchset implements the following > https://www.kernel.org/doc/Documentation/ABI/testing/sysfs-devices-soc > > Currently the required information is more or less read across the whole > SoC, but I guess we cannot change that since these are the locations > with the required information. > > There seem to be three options for the revision field: > - ROM revision (see https://community.freescale.com/docs/DOC-94802) > - ANADIG revision (ANADIG_DIGIPROC, as used for the i.MX SoC's) > - OCOTP revision > > Some numbers: > > Colibri VF61 1.1A (2N02G) > - 0x0013 > - 0x0061 > - 0x0100 > - 0x41c8 > > Colibri VF61 V1.0B (1N02G) > - 0x0011 > - 0x0061 > - 0x0100 > - 0x41c8 > > Colibri VF61 V1.0A (which is actually a VF600 SoC, no L2 cache, since > that was the only one we could buy back then, 1N02G printed on it) > - 0x0011 > - 0x0061 > - 0x0100 > - none... > > Colibri VF50 V1.0A (1N02G) > - 0x0011 > - 0x0061 > - 0x0100 > - none... > > Vybrid Tower Rev J (1N02G) > - 0x0011 > - 0x0061 > - 0x0100 > - 0x41c8 > > Read from u-boot > md.l 0x80 1 > md.l 0x40050260 1 > md.l 0x400A5090 1 > > > The ROM revision seems to differ most. So we would like to go with the > revision from the ROM register 0x80. > > Now coming to the primary question. This ROM revision register is not > really within any of the peripheral maps and I would like to access it > for the versioning information. Currently, I used ioremap like below > > ioremap(ROM_REVISION_REGISTER, SZ_1); > > which I guess probably is not the right way to do it. What would be the > correct or better way to do this? > > Also comments or feedback or any of the other parts of the patch are > also welcome. > > Some Sample outputs are below: > On Colibri VF61 V1.1A: > root@colibri-vf:/sys/devices/soc0# ls > backlight fxosc regulators sound uevent > bl_on gpio-keys revision subsystem > clk16m machinesocsxosc > family power soc_id syscon-reboot > root@colibri-vf:/sys/devices/soc0# cat revision > 0013 > root@colibri-vf:/sys/devices/soc0# cat soc_id > dbc8435c211629d4 > root@colibri-vf:/sys/devices/soc0# cat family > Freescale Vybrid VF610 > > On Colibri VF50 V1.1A: > root@colibri-vf:/sys/devices/soc0# ls > backlight machine subsystem > bl_on power sxosc > clk16m regulators syscon-reboot > family revisiontoradex,vf50_touchctrl > fxosc soc uevent > gpio-keys soc_id > root@colibri-vf:/sys/devices/soc0# cat revision > 0013 > root@colibri-vf:/sys/devices/soc0# cat soc_id > df63c12a2e2161d4 > root@colibri-vf:/sys/devices/soc0# cat family > Freescale Vybrid VF500 > root@colibri-vf:/sys/devices/soc0# cat machine > Freescale Vybrid > > Thanks & Regards, > Sanchayan Maity. > > Sanchayan Maity (2): > ARM: dts: vfxxx: Add OCOTP node > ARM: vf610: Add SoC bus support for Vybrid > > arch/arm/boot/dts/vfxxx.dtsi | 5 +++ > arch/arm/mach-imx/mach-vf610.c | 76 > +- > 2 files changed, 80 insertions(+), 1 deletion(-) > > -- > 2.4.0 > -- 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 0/2] Implement SoC bus support for Vybrid
Hello, Ping? Any inputs? On 15-05-11 10:41:37, Sanchayan Maity wrote: Hello, Currently this patchset is based of on our local branch but would like some comments before I push this to mainline through Shawn's tree. This patchset implements the following https://www.kernel.org/doc/Documentation/ABI/testing/sysfs-devices-soc Currently the required information is more or less read across the whole SoC, but I guess we cannot change that since these are the locations with the required information. There seem to be three options for the revision field: - ROM revision (see https://community.freescale.com/docs/DOC-94802) - ANADIG revision (ANADIG_DIGIPROC, as used for the i.MX SoC's) - OCOTP revision Some numbers: Colibri VF61 1.1A (2N02G) - 0x0013 - 0x0061 - 0x0100 - 0x41c8 Colibri VF61 V1.0B (1N02G) - 0x0011 - 0x0061 - 0x0100 - 0x41c8 Colibri VF61 V1.0A (which is actually a VF600 SoC, no L2 cache, since that was the only one we could buy back then, 1N02G printed on it) - 0x0011 - 0x0061 - 0x0100 - none... Colibri VF50 V1.0A (1N02G) - 0x0011 - 0x0061 - 0x0100 - none... Vybrid Tower Rev J (1N02G) - 0x0011 - 0x0061 - 0x0100 - 0x41c8 Read from u-boot md.l 0x80 1 md.l 0x40050260 1 md.l 0x400A5090 1 The ROM revision seems to differ most. So we would like to go with the revision from the ROM register 0x80. Now coming to the primary question. This ROM revision register is not really within any of the peripheral maps and I would like to access it for the versioning information. Currently, I used ioremap like below ioremap(ROM_REVISION_REGISTER, SZ_1); which I guess probably is not the right way to do it. What would be the correct or better way to do this? Also comments or feedback or any of the other parts of the patch are also welcome. Some Sample outputs are below: On Colibri VF61 V1.1A: root@colibri-vf:/sys/devices/soc0# ls backlight fxosc regulators sound uevent bl_on gpio-keys revision subsystem clk16m machinesocsxosc family power soc_id syscon-reboot root@colibri-vf:/sys/devices/soc0# cat revision 0013 root@colibri-vf:/sys/devices/soc0# cat soc_id dbc8435c211629d4 root@colibri-vf:/sys/devices/soc0# cat family Freescale Vybrid VF610 On Colibri VF50 V1.1A: root@colibri-vf:/sys/devices/soc0# ls backlight machine subsystem bl_on power sxosc clk16m regulators syscon-reboot family revisiontoradex,vf50_touchctrl fxosc soc uevent gpio-keys soc_id root@colibri-vf:/sys/devices/soc0# cat revision 0013 root@colibri-vf:/sys/devices/soc0# cat soc_id df63c12a2e2161d4 root@colibri-vf:/sys/devices/soc0# cat family Freescale Vybrid VF500 root@colibri-vf:/sys/devices/soc0# cat machine Freescale Vybrid Thanks Regards, Sanchayan Maity. Sanchayan Maity (2): ARM: dts: vfxxx: Add OCOTP node ARM: vf610: Add SoC bus support for Vybrid arch/arm/boot/dts/vfxxx.dtsi | 5 +++ arch/arm/mach-imx/mach-vf610.c | 76 +- 2 files changed, 80 insertions(+), 1 deletion(-) -- 2.4.0 -- 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/
