RE: [PATCH v4 1/4] rtc: OMAP: Add system pm_power_off to rtc driver
On Fri, Jan 11, 2013 at 15:00:03, AnilKumar, Chimata wrote:
> On Fri, Jan 11, 2013 at 14:24:46, Russ Dill wrote:
> > On Fri, Jan 11, 2013 at 12:08 AM, Russ Dill wrote:
> > > On Thu, Dec 13, 2012 at 10:03 PM, AnilKumar Ch wrote:
> > >> From: Colin Foe-Parker
> > >>
> > >> Add system power off control to rtc driver which is the in-charge
> > >> of controlling the BeagleBone system power. The power_off routine
> > >> can be hooked up to "pm_power_off" system call.
> > >>
> > >> System power off sequence:-
> > >> * Set PMIC STATUS_OFF when PMIC_POWER_EN is pulled low
> > >> * Enable PMIC_POWER_EN in rtc module
> > >> * Set rtc ALARM2 time
> > >> * Enable ALARM2 interrupt
> > >>
> > >> Signed-off-by: Colin Foe-Parker
> > >> [anilku...@ti.com: move poweroff additions to rtc driver]
> > >> Signed-off-by: AnilKumar Ch
> > >
> > > Looks good
> > >
> > > Reviewed-by: Russ Dill
> > > Acked-by: Russ Dill
> >
> > Sorry, actually, small follow up. Is there any reason this uses
> > readl/writel in some places rather than rtc_read/rtc_write?
>
> Hi Russ Dill,
>
> Thanks for the review
>
> At OMAP_RTC_PMIC_POWER_EN_EN, 32-bit value needs to modified so
> readl/writel are used. While enabling the INTERRUPTS we can change
> it to rtc_read/rtc_write but I am not seeing any advantage if we
> do that apart from consistency.
And driver has to clean-up properly to make use of rtc_read/write
APIs with proper read/write's based on the memory type. Ideally
rtc_readb/w/l and rtc_writeb/w/l should go way from the driver.
This clean-up can be done while converting the driver with platform
specific rtc_read/write.
Thanks
AnilKumar
> > >> ---
> > >> Documentation/devicetree/bindings/rtc/rtc-omap.txt |5 ++
> > >> drivers/rtc/rtc-omap.c | 74
> > >> +++-
> > >> 2 files changed, 78 insertions(+), 1 deletion(-)
> > >>
> > >> diff --git a/Documentation/devicetree/bindings/rtc/rtc-omap.txt
> > >> b/Documentation/devicetree/bindings/rtc/rtc-omap.txt
> > >> index b47aa41..8d9f4f9 100644
> > >> --- a/Documentation/devicetree/bindings/rtc/rtc-omap.txt
> > >> +++ b/Documentation/devicetree/bindings/rtc/rtc-omap.txt
> > >> @@ -6,6 +6,10 @@ Required properties:
> > >> - interrupts: rtc timer, alarm interrupts in order
> > >> - interrupt-parent: phandle for the interrupt controller
> > >>
> > >> +Optional properties:
> > >> +- ti,system-power-controller: Telling whether or not rtc is controlling
> > >> + the system power.
> > >> +
> > >> Example:
> > >>
> > >> rtc@1c23000 {
> > >> @@ -14,4 +18,5 @@ rtc@1c23000 {
> > >> interrupts = <19
> > >> 19>;
> > >> interrupt-parent = <&intc>;
> > >> + ti,system-power-controller;
> > >> };
> > >> diff --git a/drivers/rtc/rtc-omap.c b/drivers/rtc/rtc-omap.c
> > >> index 6009714..e6d4878 100644
> > >> --- a/drivers/rtc/rtc-omap.c
> > >> +++ b/drivers/rtc/rtc-omap.c
> > >> @@ -72,6 +72,14 @@
> > >> #define OMAP_RTC_KICK0_REG 0x6c
> > >> #define OMAP_RTC_KICK1_REG 0x70
> > >>
> > >> +#define OMAP_RTC_ALARM2_SECONDS_REG0x80
> > >> +#define OMAP_RTC_ALARM2_MINUTES_REG0x84
> > >> +#define OMAP_RTC_ALARM2_HOURS_REG 0x88
> > >> +#define OMAP_RTC_ALARM2_DAYS_REG 0x8c
> > >> +#define OMAP_RTC_ALARM2_MONTHS_REG 0x90
> > >> +#define OMAP_RTC_ALARM2_YEARS_REG 0x94
> > >> +#define OMAP_RTC_PMIC_REG 0x98
> > >> +
> > >> /* OMAP_RTC_CTRL_REG bit fields: */
> > >> #define OMAP_RTC_CTRL_SPLIT(1<<7)
> > >> #define OMAP_RTC_CTRL_DISABLE (1<<6)
> > >> @@ -93,15 +101,21 @@
> > >> #define OMAP_RTC_STATUS_BUSY(1<<0)
> > >>
> > >> /* OMAP_RTC_INTERRUPTS_REG bit fields: */
> > >> +#define OMAP_RTC_INTERRUPTS_IT_ALARM2 (1<<4)
> > >> #define OMAP_RTC_INTERRUPTS_IT_ALARM(1<<3)
> > >> #define OMAP_RTC_INTERRUPTS_IT_TIMER(1<<2)
> > >>
> > >> +/* OMAP_RTC_PMIC_REG bit fields: */
> > >>
RE: [PATCH v4 1/4] rtc: OMAP: Add system pm_power_off to rtc driver
On Fri, Jan 11, 2013 at 14:24:46, Russ Dill wrote:
> On Fri, Jan 11, 2013 at 12:08 AM, Russ Dill wrote:
> > On Thu, Dec 13, 2012 at 10:03 PM, AnilKumar Ch wrote:
> >> From: Colin Foe-Parker
> >>
> >> Add system power off control to rtc driver which is the in-charge
> >> of controlling the BeagleBone system power. The power_off routine
> >> can be hooked up to "pm_power_off" system call.
> >>
> >> System power off sequence:-
> >> * Set PMIC STATUS_OFF when PMIC_POWER_EN is pulled low
> >> * Enable PMIC_POWER_EN in rtc module
> >> * Set rtc ALARM2 time
> >> * Enable ALARM2 interrupt
> >>
> >> Signed-off-by: Colin Foe-Parker
> >> [anilku...@ti.com: move poweroff additions to rtc driver]
> >> Signed-off-by: AnilKumar Ch
> >
> > Looks good
> >
> > Reviewed-by: Russ Dill
> > Acked-by: Russ Dill
>
> Sorry, actually, small follow up. Is there any reason this uses
> readl/writel in some places rather than rtc_read/rtc_write?
Hi Russ Dill,
Thanks for the review
At OMAP_RTC_PMIC_POWER_EN_EN, 32-bit value needs to modified so
readl/writel are used. While enabling the INTERRUPTS we can change
it to rtc_read/rtc_write but I am not seeing any advantage if we
do that apart from consistency.
Thanks
AnilKumar
>
> >> ---
> >> Documentation/devicetree/bindings/rtc/rtc-omap.txt |5 ++
> >> drivers/rtc/rtc-omap.c | 74
> >> +++-
> >> 2 files changed, 78 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/Documentation/devicetree/bindings/rtc/rtc-omap.txt
> >> b/Documentation/devicetree/bindings/rtc/rtc-omap.txt
> >> index b47aa41..8d9f4f9 100644
> >> --- a/Documentation/devicetree/bindings/rtc/rtc-omap.txt
> >> +++ b/Documentation/devicetree/bindings/rtc/rtc-omap.txt
> >> @@ -6,6 +6,10 @@ Required properties:
> >> - interrupts: rtc timer, alarm interrupts in order
> >> - interrupt-parent: phandle for the interrupt controller
> >>
> >> +Optional properties:
> >> +- ti,system-power-controller: Telling whether or not rtc is controlling
> >> + the system power.
> >> +
> >> Example:
> >>
> >> rtc@1c23000 {
> >> @@ -14,4 +18,5 @@ rtc@1c23000 {
> >> interrupts = <19
> >> 19>;
> >> interrupt-parent = <&intc>;
> >> + ti,system-power-controller;
> >> };
> >> diff --git a/drivers/rtc/rtc-omap.c b/drivers/rtc/rtc-omap.c
> >> index 6009714..e6d4878 100644
> >> --- a/drivers/rtc/rtc-omap.c
> >> +++ b/drivers/rtc/rtc-omap.c
> >> @@ -72,6 +72,14 @@
> >> #define OMAP_RTC_KICK0_REG 0x6c
> >> #define OMAP_RTC_KICK1_REG 0x70
> >>
> >> +#define OMAP_RTC_ALARM2_SECONDS_REG0x80
> >> +#define OMAP_RTC_ALARM2_MINUTES_REG0x84
> >> +#define OMAP_RTC_ALARM2_HOURS_REG 0x88
> >> +#define OMAP_RTC_ALARM2_DAYS_REG 0x8c
> >> +#define OMAP_RTC_ALARM2_MONTHS_REG 0x90
> >> +#define OMAP_RTC_ALARM2_YEARS_REG 0x94
> >> +#define OMAP_RTC_PMIC_REG 0x98
> >> +
> >> /* OMAP_RTC_CTRL_REG bit fields: */
> >> #define OMAP_RTC_CTRL_SPLIT(1<<7)
> >> #define OMAP_RTC_CTRL_DISABLE (1<<6)
> >> @@ -93,15 +101,21 @@
> >> #define OMAP_RTC_STATUS_BUSY(1<<0)
> >>
> >> /* OMAP_RTC_INTERRUPTS_REG bit fields: */
> >> +#define OMAP_RTC_INTERRUPTS_IT_ALARM2 (1<<4)
> >> #define OMAP_RTC_INTERRUPTS_IT_ALARM(1<<3)
> >> #define OMAP_RTC_INTERRUPTS_IT_TIMER(1<<2)
> >>
> >> +/* OMAP_RTC_PMIC_REG bit fields: */
> >> +#define OMAP_RTC_PMIC_POWER_EN_EN (1<<16)
> >> +
> >> /* OMAP_RTC_KICKER values */
> >> #defineKICK0_VALUE 0x83e70b13
> >> #defineKICK1_VALUE 0x95a4f1e0
> >>
> >> #defineOMAP_RTC_HAS_KICKER 0x1
> >>
> >> +#define SHUTDOWN_TIME_SEC 2
> >> +
> >> static void __iomem*rtc_base;
> >>
> >> #define rtc_read(addr) readb(rtc_base + (addr))
> >> @@ -290,6 +304,56 @@ static int omap_rtc_set_alarm(struct device *dev,
> >> struct rtc_wkalrm *alm)
> >> return 0;
> >> }
> >>
> >> +/*
> >> + * rtc_power_off: Set the pmic power off sequence. The RTC generates
> >> + * pmic_pwr_enable control, which can be used to control an external
> >> + * PMIC.
> >> + */
> >> +static void rtc_power_off(void)
> >> +{
> >> + u32 val;
> >> + struct rtc_time tm;
> >> + unsigned long time;
> >> +
> >> + /* Set PMIC power enable */
> >> + val = readl(rtc_base + OMAP_RTC_PMIC_REG);
> >> + writel(val | OMAP_RTC_PMIC_POWER_EN_EN, rtc_base +
> >> OMAP_RTC_PMIC_REG);
> >> +
> >> + /* Read rtc time */
> >> + omap_rtc_read_time(NULL, &tm);
> >> +
> >> + /* Convert Gregorian date to seconds since 01-01-1970 00:00:00 */
> >> + rtc_tm_to_time(&tm, &time);
> >> +
> >> + /* Add shutdown time to the current value */
> >> + time += SHUTDOWN_TIME_SEC;
> >> +
> >> + /* Convert seconds since 01-01-1970 00:00:00 to Gregorian date */
> >> + rtc_time_to_tm(time, &tm);
> >> +
> >> + if (tm2bcd(&tm)
RE: [PATCH 0/3] can: Add D_CAN raminit support to am335x-evm
On Wed, Jan 09, 2013 at 17:46:37, Cousson, Benoit wrote: > Hi Anil, > > On 11/14/2012 07:08 PM, AnilKumar Ch wrote: > > This patch series adds d_can raminit support to c_can/d_can driver, > > which is required to init/de-init D_CAN message RAM (holds message > > objects). Added corresponding DT changes to get resource of RAMINIT > > register and device instance. > > > > These patches were tested on AM335x-EVM along with pinctrl data addition > > patch, which is not added to am335x-evm.dts (only supports CPLD profile#0) > > because d_can1 is supported under CPLD profile#1. > > > > AnilKumar Ch (3): > > can: c_can: Add d_can raminit support > > ARM: dts: AM33XX: Add d_can instances to aliases > > ARM: dts: AM33XX: Add memory resource to d_can node > > I've just applied both DTS patches with Acked-by from Marc in my > for_3.9/dts branch. > Hi Benoit, Thanks much Thanks AnilKumar ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: [PATCH v4 0/4] pm: Add power off control
On Fri, Dec 14, 2012 at 11:33:13, AnilKumar, Chimata wrote: > Add PM power_off control to rtc driver and PMIC status is set to > STATUS_OFF to shutdown PMIC if PWR_EN is toggled by RTC module. > > System power off sequence:- > * Set PMIC STATUS_OFF when PMIC_POWER_EN is pulled low > * Enable PMIC_POWER_EN in rtc module > * Set rtc ALARM2 time > * Enable ALARM2 interrupt > > These patches were tested on am335x-bone (BeagleBone). These patchs > are based on linux-next. Hi All, Happy New Year Gentle ping to this series Thanks AnilKumar > Changes from v3: > - TPS65217 mfd driver changes are accepted so dropped > from this series. > - With recent changes in the kernel, reboot_mutex held BUG() > is not seen so removed while(1); from rtc_power_off. > - Removed spinlock before while(1); which is not necessary. > > Changes from v2: > - Incorporated Kevin's comment on v2 > * Enabled RTC in ompa2plus_defconfig > > Changes from v1: > - Incorporated Vaibhav's comments on v1 > * Changed the time rollover logic with the help of > rtc-lib APIs > > AnilKumar Ch (3): > ARM: dts: AM33XX: Set pmic-shutdown-controller for BeagleBone > ARM: dts: AM33XX: Enable system power off control in am335x-bone > ARM: OMAP2+: omap2plus_defconfig: Enable RTC support > > Colin Foe-Parker (1): > rtc: OMAP: Add system pm_power_off to rtc driver > > Documentation/devicetree/bindings/rtc/rtc-omap.txt |5 ++ > arch/arm/boot/dts/am335x-bone.dts |6 ++ > arch/arm/configs/omap2plus_defconfig |1 + > drivers/rtc/rtc-omap.c | 74 > +++- > 4 files changed, 85 insertions(+), 1 deletion(-) > > -- > 1.7.9.5 > > ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: [PATCH 3/3] ARM: dts: AM33XX: Add memory resource to d_can node
On Wed, Nov 21, 2012 at 13:50:26, Marc Kleine-Budde wrote: > On 11/14/2012 07:08 PM, AnilKumar Ch wrote: > > Add a new address space/memory resource to d_can device tree node. D_CAN > > RAM initialization is achieved through RAMINIT register which is part of > > AM33XX control module address space. D_CAN RAM init or de-init should be > > done by writing instance corresponding value to control module register. > > > > Till we have a separate control module driver to write to control module, > > d_can driver will handle the register writes to control module by itself. > > So a new address space to represent this control module register is added > > to d_can driver. > > > > Signed-off-by: AnilKumar Ch > > Acked-by: Marc Kleine-Budde > Hi Tony/Benoit, Could you please pull this in? Thanks AnilKumar ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: [PATCH 2/3] ARM: dts: AM33XX: Add d_can instances to aliases
On Tue, Nov 20, 2012 at 16:21:42, Marc Kleine-Budde wrote: > On 11/14/2012 07:08 PM, AnilKumar Ch wrote: > > Add d_can instances to aliases node to get the D_CAN instance number > > from the driver. To initialize D_CAN message RAM, corresponding instance > > number is required. > > > > To initialize instance 0 message RAM then 0x1 should be written and for > > instance 1 message RAM, 0x2 should be written to control module register. > > > > With device-tree framework ip instance number is "-1" by default for all > > instances. To get device id/instance number then modules should be added > > to DT "aliases" node. of_alias_get_id() gives the device id number based > > on number of alias nodes present in "aliases node". > > > > Signed-off-by: AnilKumar Ch > > Acked-by: Marc Kleine-Budde Hi Tony/Benoit, Could you please pull this in? Thanks AnilKumar ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: [PATCH RESEND] ARM: dts: AM33XX: Rename I2C and GPIO nodes
On Wed, Nov 21, 2012 at 17:22:17, AnilKumar, Chimata wrote:
> Rename I2C and GPIO nodes according to AM33XX TRM. According to
> AM33XX TRM device instances are starting from "0" like i2c0, i2c1
> and i2c3.
>
> Signed-off-by: Pantelis Antoniou
> [pa...@antoniou-consulting.com: initial patch by pantelis's]
> Signed-off-by: AnilKumar Ch
> ---
> Changes from first version:
> - Updated pantelis's patch
> * Modified based on linux-omap/master
> * Added GPIO nodes renaming as well
Hi Tony/Benoit,
If there are no comments could you please pull this patch?
Thanks
AnilKumar
> arch/arm/boot/dts/am335x-bone.dts | 10 +-
> arch/arm/boot/dts/am335x-evm.dts | 18 +-
> arch/arm/boot/dts/am335x-evmsk.dts | 18 +-
> arch/arm/boot/dts/am33xx.dtsi | 14 +++---
> 4 files changed, 30 insertions(+), 30 deletions(-)
>
> diff --git a/arch/arm/boot/dts/am335x-bone.dts
> b/arch/arm/boot/dts/am335x-bone.dts
> index 2c33888..77b4352 100644
> --- a/arch/arm/boot/dts/am335x-bone.dts
> +++ b/arch/arm/boot/dts/am335x-bone.dts
> @@ -43,7 +43,7 @@
> status = "okay";
> };
>
> - i2c1: i2c@44e0b000 {
> + i2c0: i2c@44e0b000 {
> status = "okay";
> clock-frequency = <40>;
>
> @@ -59,27 +59,27 @@
>
> led@2 {
> label = "beaglebone:green:heartbeat";
> - gpios = <&gpio2 21 0>;
> + gpios = <&gpio1 21 0>;
> linux,default-trigger = "heartbeat";
> default-state = "off";
> };
>
> led@3 {
> label = "beaglebone:green:mmc0";
> - gpios = <&gpio2 22 0>;
> + gpios = <&gpio1 22 0>;
> linux,default-trigger = "mmc0";
> default-state = "off";
> };
>
> led@4 {
> label = "beaglebone:green:usr2";
> - gpios = <&gpio2 23 0>;
> + gpios = <&gpio1 23 0>;
> default-state = "off";
> };
>
> led@5 {
> label = "beaglebone:green:usr3";
> - gpios = <&gpio2 24 0>;
> + gpios = <&gpio1 24 0>;
> default-state = "off";
> };
> };
> diff --git a/arch/arm/boot/dts/am335x-evm.dts
> b/arch/arm/boot/dts/am335x-evm.dts
> index 9f65f17..705a9c5 100644
> --- a/arch/arm/boot/dts/am335x-evm.dts
> +++ b/arch/arm/boot/dts/am335x-evm.dts
> @@ -51,7 +51,7 @@
> status = "okay";
> };
>
> - i2c1: i2c@44e0b000 {
> + i2c0: i2c@44e0b000 {
> status = "okay";
> clock-frequency = <40>;
>
> @@ -60,7 +60,7 @@
> };
> };
>
> - i2c2: i2c@4802a000 {
> + i2c1: i2c@4802a000 {
> status = "okay";
> clock-frequency = <10>;
>
> @@ -123,12 +123,12 @@
> debounce-delay-ms = <5>;
> col-scan-delay-us = <2>;
>
> - row-gpios = <&gpio2 25 0/* Bank1, pin25 */
> - &gpio2 26 0/* Bank1, pin26 */
> - &gpio2 27 0>; /* Bank1, pin27 */
> + row-gpios = <&gpio1 25 0/* Bank1, pin25 */
> + &gpio1 26 0/* Bank1, pin26 */
> + &gpio1 27 0>; /* Bank1, pin27 */
>
> - col-gpios = <&gpio2 21 0/* Bank1, pin21 */
> - &gpio2 22 0>; /* Bank1, pin22 */
> + col-gpios = <&gpio1 21 0/* Bank1, pin21 */
> + &gpio1 22 0>; /* Bank1, pin22 */
>
> linux,keymap = <0x008b /* MENU */
> 0x019e /* BACK */
> @@ -147,14 +147,14 @@
> switch@9 {
> label = "volume-up";
> linux,code = <115>;
> - gpios = <&gpio1 2 1>;
> +
RE: [rtc-linux] [PATCH v3 1/5] rtc: OMAP: Add system pm_power_off to rtc driver
On Wed, Nov 28, 2012 at 16:42:26, Russell King - ARM Linux wrote:
> On Tue, Nov 27, 2012 at 03:42:39PM -0800, Andrew Morton wrote:
> > > + /* Do not allow to execute any other task */
> > > + spin_lock_irqsave(&lock, flags);
> > > + while (1);
> >
> > I suspect this doesn't do what you want it to do.
> >
> > Firstly, please provide adequate code comments here so that code
> > readers do not also need to be mind readers.
> >
> > If you want to stop this CPU dead in its tracks (why?) then
> >
> > local_irq_disable();
> > while (1)
> > ; /* Note correct code layout */
> >
> > will do it. But it means that the NMI watchdog (if present) will come
> > along and whack the machine in the head a few seconds later. And this
> > does nothing to stop other CPUs.
> >
> > But not being a mind reader, I'm really at a loss to suggest what
> > should be done here.
Here intention is to disable all interrupts between rtc power_off request
till system actually went to power off mode, max 2 seconds based on when
the AM335x RTC alarm2 expires. The idea was that since the system is
going to shutdown, it is better to not process any more interrupts.
> It's hooking into the pm_power_off hook, which is called from kernel/sys.c
> via arch code. We will have already stopped all other CPUs at this point.
>
> Why there's that while (1) there I don't know; when pm_power_off is not
> hooked, we don't do anything like that - and what will happen in that
> case is we'll return all the way back to sys_reboot(), which will call
> do_exit(0) on us.
When testing with v3.7-rc7, I saw the following BUG() triggered when I did
not use a while(1). Logs below.
Before calling do_exit(0), there is a reboot_mutex lock that is taken in
reboot syscall. do_exit() function is , checking for any locks held by the
processor and if yes then it is printing a BUG_ON from print_held_locks_bug()
function along with the locks held information.
Even though the BUG triggers, the system is going to power off because
The hardware has been triggered.
(log)
[root@arago /]# poweroff
The system is going down NOW!
Sent SIGTERM to all processes
Sent SIGKILL to all processes
Requesting system poweroff
[ 32.125900] Disabling non-boot CPUs ...
[ 32.130155] Power down.
[ 32.132741] System will go to power_off state in approx. 2 secs
[ 32.139994]
[ 32.141575] =
[ 32.146564] [ BUG: lock held at task exit time! ]
[ 32.151522] 3.7.0-rc7-00015-g3f8bbe0 #32 Not tainted
[ 32.156721] -
[ 32.161676] init/622 is exiting with locks still held!
[ 32.167085] 1 lock held by init/622:
[ 32.170831] #0: (reboot_mutex){+.+...}, at: []
sys_reboot+0xf4/0x1e8
[ 32.178679]
[ 32.178679] stack backtrace:
[ 32.183305] [] (unwind_backtrace+0x0/0xf0) from []
(do_exit+0x488/0x7e8)
[ 32.192183] [] (do_exit+0x488/0x7e8) from []
(sys_reboot+0x100/0x1e8)
[ 32.200797] [] (sys_reboot+0x100/0x1e8) from []
(ret_fast_syscall+0x0/0x3c)
=
It is not clear to me where reboot_mutex should have been unlocked before
debug_check_no_locks_held() is called in do_exit()?
Note: In latest linux-next tip I am *not* seeing this error. I have not
bisected yet to see what fixed this. Since with latest linux-next I am
not seeing an issue, I will remove spinlock and the while(1). Hope that
will make the patch more acceptable.
> I don't see a problem with that, and I don't see why we need to spin
> (without any power saving too) waiting for some event. If we've called
> sys_reboot with LINUX_REBOOT_CMD_POWER_OFF, we'd better have already
> killed most of userspace off by that time anyway.
>
I did a search of various power-off implementations in *arch/arm*.
Those that use a while(1):
=
arch/arm/mach-at91/board-gsia18s.c
arch/arm/mach-at91/setup.c doesn't
arch/arm/mach-cns3xxx/cns3420vb.c
arch/arm/mach-iop32x/glantank.c
arch/arm/mach-iop32x/iq31244.c
arch/arm/mach-iop32x/n2100.c local_irq_disable() and then a while(1)
arch/arm/mach-kirkwood/board-lsxl.c
arch/arm/mach-highbank/highbank.c does while(1) cpu_do_idle()
Those that don't have a while(1):
arch/arm/mach-imx/mach-mx31moboard.c
arch/arm/mach-iop32x/em7210.c
Doesn't have a while(1) but setting a gpio so presumably kills the system
immediately:
===
arch/arm/mach-ixp4xx/dsmg600-setup.c
arch/arm/mach-ixp4xx/nas100d-setup.c
arch/arm/mach-ixp4xx/nslu2-setup.c
arch/arm/mach-kirkwood/board-dnskw.c
arch/arm/mach-kirkwood/board-ib62x0.c
arch/arm/mach-kirkwood/board-dnskw.c
arch/arm/mach-kirkwood/board-ib62x0.c
arch/arm/mach-kirkwood/d2net_v2-setup.c
arch/arm/mach-kirkwood/netspace_v2-setup.c
arch/arm/mach-kirkwood/netxbig_v2-setup.c
arch/arm/mach-kirkwood/t5325-setup.c
arch/arm/mach-orion5x/dns323-setup.c
arch/arm/mach-vexpress/v2m.c take a spinlock, not sure when it will shut down
arch/arm/mach-kirkwood/board-ts219.c sends a character to PIC
RE: [PATCH v3 3/5] ARM: dts: AM33XX: Set pmic-shutdown-controller for BeagleBone
On Tue, Nov 20, 2012 at 15:18:45, AnilKumar, Chimata wrote:
> Set ti,pmic-shutdown-controller for BeagleBone in am335x-bone.dts
> file, this flag is used by the driver to set tps65217 PMIC status
> to OFF when PWR_EN toggle.
Hi Tony,
Could you please pull these 3 patches? (3/5, 4/5 and 5/5)
Thanks
AnilKumar
> Signed-off-by: AnilKumar Ch
> ---
> arch/arm/boot/dts/am335x-bone.dts |2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/arch/arm/boot/dts/am335x-bone.dts
> b/arch/arm/boot/dts/am335x-bone.dts
> index 2c33888..1d55190 100644
> --- a/arch/arm/boot/dts/am335x-bone.dts
> +++ b/arch/arm/boot/dts/am335x-bone.dts
> @@ -88,6 +88,8 @@
> /include/ "tps65217.dtsi"
>
> &tps {
> + ti,pmic-shutdown-controller;
> +
> regulators {
> dcdc1_reg: regulator@0 {
> regulator-always-on;
> --
> 1.7.9.5
>
>
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: [PATCH v3 1/5] rtc: OMAP: Add system pm_power_off to rtc driver
+Andrew Morton
On Tue, Nov 20, 2012 at 15:18:43, AnilKumar, Chimata wrote:
> From: Colin Foe-Parker
>
> Add system power off control to rtc driver which is the in-charge
> of controlling the BeagleBone system power. The power_off routine
> can be hooked up to "pm_power_off" system call.
>
> System power off sequence:-
> * Set PMIC STATUS_OFF when PMIC_POWER_EN is pulled low
> * Enable PMIC_POWER_EN in rtc module
> * Set rtc ALARM2 time
> * Enable ALARM2 interrupt
>
> Added while (1); after the above steps to make sure that no other
> process acquire cpu. Otherwise we might see an unexpected behaviour
> because we are shutting down all the power rails of SoC except RTC.
Hi All,
ACK from reviewers will help this patch get in.
Andrew,
Could you please pull this patch?
Thanks
AnilKumar
> Signed-off-by: Colin Foe-Parker
> [anilku...@ti.com: move poweroff additions to rtc driver]
> Signed-off-by: AnilKumar Ch
> ---
> Documentation/devicetree/bindings/rtc/rtc-omap.txt |5 ++
> drivers/rtc/rtc-omap.c | 81
> +++-
> 2 files changed, 85 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/rtc/rtc-omap.txt
> b/Documentation/devicetree/bindings/rtc/rtc-omap.txt
> index b47aa41..8d9f4f9 100644
> --- a/Documentation/devicetree/bindings/rtc/rtc-omap.txt
> +++ b/Documentation/devicetree/bindings/rtc/rtc-omap.txt
> @@ -6,6 +6,10 @@ Required properties:
> - interrupts: rtc timer, alarm interrupts in order
> - interrupt-parent: phandle for the interrupt controller
>
> +Optional properties:
> +- ti,system-power-controller: Telling whether or not rtc is controlling
> + the system power.
> +
> Example:
>
> rtc@1c23000 {
> @@ -14,4 +18,5 @@ rtc@1c23000 {
> interrupts = <19
> 19>;
> interrupt-parent = <&intc>;
> + ti,system-power-controller;
> };
> diff --git a/drivers/rtc/rtc-omap.c b/drivers/rtc/rtc-omap.c
> index 6009714..c31f93a 100644
> --- a/drivers/rtc/rtc-omap.c
> +++ b/drivers/rtc/rtc-omap.c
> @@ -72,6 +72,14 @@
> #define OMAP_RTC_KICK0_REG 0x6c
> #define OMAP_RTC_KICK1_REG 0x70
>
> +#define OMAP_RTC_ALARM2_SECONDS_REG 0x80
> +#define OMAP_RTC_ALARM2_MINUTES_REG 0x84
> +#define OMAP_RTC_ALARM2_HOURS_REG0x88
> +#define OMAP_RTC_ALARM2_DAYS_REG 0x8c
> +#define OMAP_RTC_ALARM2_MONTHS_REG 0x90
> +#define OMAP_RTC_ALARM2_YEARS_REG0x94
> +#define OMAP_RTC_PMIC_REG0x98
> +
> /* OMAP_RTC_CTRL_REG bit fields: */
> #define OMAP_RTC_CTRL_SPLIT (1<<7)
> #define OMAP_RTC_CTRL_DISABLE(1<<6)
> @@ -93,15 +101,21 @@
> #define OMAP_RTC_STATUS_BUSY(1<<0)
>
> /* OMAP_RTC_INTERRUPTS_REG bit fields: */
> +#define OMAP_RTC_INTERRUPTS_IT_ALARM2 (1<<4)
> #define OMAP_RTC_INTERRUPTS_IT_ALARM(1<<3)
> #define OMAP_RTC_INTERRUPTS_IT_TIMER(1<<2)
>
> +/* OMAP_RTC_PMIC_REG bit fields: */
> +#define OMAP_RTC_PMIC_POWER_EN_EN (1<<16)
> +
> /* OMAP_RTC_KICKER values */
> #define KICK0_VALUE 0x83e70b13
> #define KICK1_VALUE 0x95a4f1e0
>
> #define OMAP_RTC_HAS_KICKER 0x1
>
> +#define SHUTDOWN_TIME_SEC2
> +
> static void __iomem *rtc_base;
>
> #define rtc_read(addr) readb(rtc_base + (addr))
> @@ -290,6 +304,63 @@ static int omap_rtc_set_alarm(struct device *dev, struct
> rtc_wkalrm *alm)
> return 0;
> }
>
> +/*
> + * rtc_power_off: Set the pmic power off sequence. The RTC generates
> + * pmic_pwr_enable control, which can be used to control an external
> + * PMIC.
> + */
> +static void rtc_power_off(void)
> +{
> + u32 val;
> + struct rtc_time tm;
> + spinlock_t lock;
> + unsigned long flags, time;
> +
> + spin_lock_init(&lock);
> +
> + /* Set PMIC power enable */
> + val = readl(rtc_base + OMAP_RTC_PMIC_REG);
> + writel(val | OMAP_RTC_PMIC_POWER_EN_EN, rtc_base + OMAP_RTC_PMIC_REG);
> +
> + /* Read rtc time */
> + omap_rtc_read_time(NULL, &tm);
> +
> + /* Convert Gregorian date to seconds since 01-01-1970 00:00:00 */
> + rtc_tm_to_time(&tm, &time);
> +
> + /* Add shutdown time to the current value */
> + time += SHUTDOWN_TIME_SEC;
> +
> + /* Convert seconds since 01-01-1970 00:00:00 to Gregorian date */
> + rtc_time_to_tm(time, &tm);
> +
> + if (tm2bcd(&tm) < 0)
> + return;
> +
> + pr_info("System will go to power_off state in approx. %d
RE: [PATCH v3 2/5] mfd: tps65217: Set PMIC to shutdown on PWR_EN toggle
On Wed, Nov 21, 2012 at 19:17:58, Samuel Ortiz wrote: > Hi Anilkumar, > > On Tue, Nov 20, 2012 at 03:18:44PM +0530, AnilKumar Ch wrote: > > From: Colin Foe-Parker > > > > Set tps65217 PMIC status to OFF if power enable toggle is supported. > > By setting this bit to 1 to enter PMIC to OFF state when PWR_EN pin > > is pulled low. Also adds a DT flag to specify that device pmic > > supports shutdown control or not. > > > > Signed-off-by: Colin Foe-Parker > > [anilku...@ti.com: move the additions to tps65217 MFD driver] > > Signed-off-by: AnilKumar Ch > > --- > > .../devicetree/bindings/regulator/tps65217.txt |4 > > drivers/mfd/tps65217.c | 12 > > 2 files changed, 16 insertions(+) > Applied, thanks. > I suppose you're not expecting the whole patchset to go through one tree ? > Hi Samuel, Thanks much. Yes, I will request the corresponding owners to pick rest of the patches. Thanks AnilKumar ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: [PATCH v2] can: c_can: Add d_can raminit support
On Wed, Nov 21, 2012 at 14:18:56, Marc Kleine-Budde wrote:
> On 11/21/2012 06:44 AM, AnilKumar Ch wrote:
> > Add D_CAN raminit support to C_CAN driver to enable D_CAN RAM,
> > which holds all the message objects during transmission or
> > receiving of data. This initialization/de-initialization should
> > be done in synchronous with D_CAN clock.
> >
> > In case of AM335X-EVM (current user of D_CAN driver) message RAM is
> > controlled through control module register for both instances. So
> > control module register details is required to initialization or
> > de-initialization of message RAM according to instance number.
> >
> > Control module memory resource is obtained from D_CAN dt node and
> > instance number obtained from device tree aliases node.
> >
> > This patch was tested on AM335x-EVM along with pinctrl data addition
> > patch, d_can dt aliases addition and control module data addition.
> > pinctrl data addition is not added to am335x-evm.dts (only supports
> > CPLD profile#0) because d_can1 is supported under CPLD profile#1.
> >
> > Signed-off-by: AnilKumar Ch
> > ---
>
> [...]
>
> > @@ -178,6 +195,20 @@ static int __devinit c_can_plat_probe(struct
> > platform_device *pdev)
> > priv->can.ctrlmode_supported |= CAN_CTRLMODE_3_SAMPLES;
> > priv->read_reg = c_can_plat_read_reg_aligned_to_16bit;
> > priv->write_reg = c_can_plat_write_reg_aligned_to_16bit;
> > +
> > + if (pdev->dev.of_node)
> > + priv->instance = pdev->id < 0 ?
> > + of_alias_get_id(pdev->dev.of_node, "d_can") :
> > + pdev->id;
>
> This wouldn't work with non DT kernels, what about:
>
> if (pdev->dev.of_node)
> priv->instance = of_alias_get_id(pdev->dev.of_node, "d_can");
> else
> priv->instance = pdev->id;
>
I completely forgot this case, yes this is the correct way of doing it.
Thanks
AnilKumar
> > +
> > + res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> > + priv->raminit_ctrlreg =
> > + devm_request_and_ioremap(&pdev->dev, res);
> > + if (!priv->raminit_ctrlreg || priv->instance < 0) {
> > + dev_info(&pdev->dev, "control memory is not used for
> > raminit\n");
> > + break;
> > + }
> > + priv->ram_init = c_can_hw_raminit;
> > break;
> > default:
> > ret = -EINVAL;
> >
>
>
> --
> Pengutronix e.K. | Marc Kleine-Budde |
> Industrial Linux Solutions| Phone: +49-231-2826-924 |
> Vertretung West/Dortmund | Fax: +49-5121-206917- |
> Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
>
>
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: [PATCH v2] can: c_can: Add d_can raminit support
On Wed, Nov 21, 2012 at 13:57:17, Marc Kleine-Budde wrote:
> On 11/21/2012 06:44 AM, AnilKumar Ch wrote:
> > Add D_CAN raminit support to C_CAN driver to enable D_CAN RAM,
> > which holds all the message objects during transmission or
> > receiving of data. This initialization/de-initialization should
> > be done in synchronous with D_CAN clock.
> >
> > In case of AM335X-EVM (current user of D_CAN driver) message RAM is
> > controlled through control module register for both instances. So
> > control module register details is required to initialization or
> > de-initialization of message RAM according to instance number.
> >
> > Control module memory resource is obtained from D_CAN dt node and
> > instance number obtained from device tree aliases node.
> >
> > This patch was tested on AM335x-EVM along with pinctrl data addition
> > patch, d_can dt aliases addition and control module data addition.
> > pinctrl data addition is not added to am335x-evm.dts (only supports
> > CPLD profile#0) because d_can1 is supported under CPLD profile#1.
> >
> > Signed-off-by: AnilKumar Ch
> > ---
> > Changes from v1:
> > - Incorporated Marc's comments on v1
> > * sanity check moved to c_can_probe() from c_can_hw_raminit()
> > * device instance is assigned using conditional operator
> > * Changed warning to info to tell control module is not
> > used for raminit if there is no second IORESOURCE_MEM
> > - Dropped dt patches
> > * No changes from v1
> > * Those will go to linux-omap/master
> >
> > drivers/net/can/c_can/c_can.c | 12
> > drivers/net/can/c_can/c_can.h |3 +++
> > drivers/net/can/c_can/c_can_platform.c | 33
> > +++-
> > 3 files changed, 47 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/can/c_can/c_can.c b/drivers/net/can/c_can/c_can.c
> > index e5180df..c15830c 100644
> > --- a/drivers/net/can/c_can/c_can.c
> > +++ b/drivers/net/can/c_can/c_can.c
> > @@ -233,6 +233,12 @@ static inline void c_can_pm_runtime_put_sync(const
> > struct c_can_priv *priv)
> > pm_runtime_put_sync(priv->device);
> > }
> >
> > +static inline void c_can_reset_ram(const struct c_can_priv *priv, bool
> > enable)
> > +{
> > + if (priv->ram_init)
> > + priv->ram_init(priv, enable);
> > +}
> > +
> > static inline int get_tx_next_msg_obj(const struct c_can_priv *priv)
> > {
> > return (priv->tx_next & C_CAN_NEXT_MSG_OBJ_MASK) +
> > @@ -1090,6 +1096,7 @@ static int c_can_open(struct net_device *dev)
> > struct c_can_priv *priv = netdev_priv(dev);
> >
> > c_can_pm_runtime_get_sync(priv);
> > + c_can_reset_ram(priv, true);
> >
> > /* open the can device */
> > err = open_candev(dev);
> > @@ -1118,6 +1125,7 @@ static int c_can_open(struct net_device *dev)
> > exit_irq_fail:
> > close_candev(dev);
> > exit_open_fail:
> > + c_can_reset_ram(priv, false);
> > c_can_pm_runtime_put_sync(priv);
> > return err;
> > }
> > @@ -1131,6 +1139,8 @@ static int c_can_close(struct net_device *dev)
> > c_can_stop(dev);
> > free_irq(dev->irq, dev);
> > close_candev(dev);
> > +
> > + c_can_reset_ram(priv, false);
> > c_can_pm_runtime_put_sync(priv);
> >
> > return 0;
> > @@ -1188,6 +1198,7 @@ int c_can_power_down(struct net_device *dev)
> >
> > c_can_stop(dev);
> >
> > + c_can_reset_ram(priv, false);
> > c_can_pm_runtime_put_sync(priv);
> >
> > return 0;
> > @@ -1206,6 +1217,7 @@ int c_can_power_up(struct net_device *dev)
> > WARN_ON(priv->type != BOSCH_D_CAN);
> >
> > c_can_pm_runtime_get_sync(priv);
> > + c_can_reset_ram(priv, true);
> >
> > /* Clear PDR and INIT bits */
> > val = priv->read_reg(priv, C_CAN_CTRL_EX_REG);
> > diff --git a/drivers/net/can/c_can/c_can.h b/drivers/net/can/c_can/c_can.h
> > index e5ed41d..419de5c 100644
> > --- a/drivers/net/can/c_can/c_can.h
> > +++ b/drivers/net/can/c_can/c_can.h
> > @@ -169,6 +169,9 @@ struct c_can_priv {
> > void *priv; /* for board-specific data */
> > u16 irqstatus;
> > enum c_can_dev_id type;
> > + u32 __iomem *raminit_ctrlreg;
> > + unsigned int instance;
> > + void (*ram_init) (const struct c_can_priv *priv, bool enable);
> > };
> >
> > struct net_device *alloc_c_can_dev(void);
> > diff --git a/drivers/net/can/c_can/c_can_platform.c
> > b/drivers/net/can/c_can/c_can_platform.c
> > index ee141613..d1c31c8 100644
> > --- a/drivers/net/can/c_can/c_can_platform.c
> > +++ b/drivers/net/can/c_can/c_can_platform.c
> > @@ -38,6 +38,8 @@
> >
> > #include "c_can.h"
> >
> > +#define CAN_RAMINIT_START_MASK(i) (1 << (i))
> > +
> > /*
> > * 16-bit c_can registers can be arranged differently in the memory
> > * architecture of different implementations. For example: 16-bit
> > @@ -68,6 +70,21 @@ static void c_can_plat_write_reg_aligned_to_32bit(struct
> > c_can_priv *priv,
> > writew(val, priv->base + 2
RE: [PATCH RESEND v6] Input: matrix-keypad - Add device tree support
On Thu, Nov 15, 2012 at 19:43:08, AnilKumar, Chimata wrote:
> Add device tree support to matrix keypad driver and usage details
> are added to device tree documentation. Also modified the driver
> according to recent update of matrix_keypad_build_keymap(), which
> automatically allocate memory for keymap. Driver was tested on AM335x
> EVM.
>
> Signed-off-by: AnilKumar Ch
> Signed-off-by: Dmitry Torokhov
> ---
> This patch was tested on AM335x-EVM and based on linux-next, cleanly
> applies on top of "input/next"
Hi Dmity,
If there are no further comments on this patch, could you please
take this in?
Thanks
AnilKumar
>
> Changes from v6:
> - Added missed DT binding document.
>
> Changes from v5:
> - Updated based on Dmitry's patch.
> * Modified the driver acc. to matrix_keypad_build_keymap()
> recent update.
> * Combined row_gpios & col_gpios devm_kzalloc's to single
> devm_kzalloc.
> * Added proper return values for different failures in
> matrix_keypad_parse_dt().
>
> Changes from v4:
> - Removed clustered-irq support through DT. This should be
> added if any platform requires in the future.
>
> Changes from v3:
> - Incorporated Stephen Warren's review comments on v3
> * Added description to clustered-irq-flags binding
>
> Changes from v2:
> - Incorporated Stephen Warren's review comments on v2
> * Renamed the bindings file to gpio-matrix-keypad.txt
> * Added description to clustered-irq binding
>
> Changes from v1:
> - Incorporated Rob's review comments on v1
> * Changed matix-keypad compatible to gpio-matrix-keypad
> * Removed unnecessary props (num of gpios)
> * Used of_match_ptr()
> - Removed first patch based on Dmitry's comments on v1
>
> .../bindings/input/gpio-matrix-keypad.txt | 46
> drivers/input/keyboard/matrix_keypad.c | 119
>
> 2 files changed, 143 insertions(+), 22 deletions(-)
> create mode 100644
> Documentation/devicetree/bindings/input/gpio-matrix-keypad.txt
>
> diff --git a/Documentation/devicetree/bindings/input/gpio-matrix-keypad.txt
> b/Documentation/devicetree/bindings/input/gpio-matrix-keypad.txt
> new file mode 100644
> index 000..ead641c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/gpio-matrix-keypad.txt
> @@ -0,0 +1,46 @@
> +* GPIO driven matrix keypad device tree bindings
> +
> +GPIO driven matrix keypad is used to interface a SoC with a matrix keypad.
> +The matrix keypad supports multiple row and column lines, a key can be
> +placed at each intersection of a unique row and a unique column. The matrix
> +keypad can sense a key-press and key-release by means of GPIO lines and
> +report the event using GPIO interrupts to the cpu.
> +
> +Required Properties:
> +- compatible:Should be "gpio-matrix-keypad"
> +- row-gpios: List of gpios used as row lines. The gpio specifier
> + for this property depends on the gpio controller to
> + which these row lines are connected.
> +- col-gpios: List of gpios used as column lines. The gpio specifier
> + for this property depends on the gpio controller to
> + which these column lines are connected.
> +- linux,keymap: The definition can be found at
> + bindings/input/matrix-keymap.txt
> +
> +Optional Properties:
> +- linux,no-autorepeat: do no enable autorepeat feature.
> +- linux,wakeup: use any event on keypad as wakeup event.
> +- debounce-delay-ms: debounce interval in milliseconds
> +- col-scan-delay-us: delay, measured in microseconds, that is needed
> + before we can scan keypad after activating column gpio
> +
> +Example:
> + matrix-keypad {
> + compatible = "gpio-matrix-keypad";
> + debounce-delay-ms = <5>;
> + col-scan-delay-us = <2>;
> +
> + row-gpios = <&gpio2 25 0
> + &gpio2 26 0
> + &gpio2 27 0>;
> +
> + col-gpios = <&gpio2 21 0
> + &gpio2 22 0>;
> +
> + linux,keymap = <0x008B
> + 0x019E
> + 0x0269
> + 0x0001006A
> + 0x0101001C
> + 0x0201006C>;
> + };
> diff --git a/drive
RE: [PATCH 3/3] ARM: dts: AM33XX: Add memory resource to d_can node
On Tue, Nov 20, 2012 at 15:56:32, Marc Kleine-Budde wrote: > On 11/20/2012 11:23 AM, AnilKumar, Chimata wrote: > > On Tue, Nov 20, 2012 at 15:43:04, Marc Kleine-Budde wrote: > >> On 11/14/2012 07:08 PM, AnilKumar Ch wrote: > >>> Add a new address space/memory resource to d_can device tree node. D_CAN > >>> RAM initialization is achieved through RAMINIT register which is part of > >>> AM33XX control module address space. D_CAN RAM init or de-init should be > >>> done by writing instance corresponding value to control module register. > >>> > >>> Till we have a separate control module driver to write to control module, > >>> d_can driver will handle the register writes to control module by itself. > >>> So a new address space to represent this control module register is added > >>> to d_can driver. > >>> > >>> Signed-off-by: AnilKumar Ch > >> > >> This does not apply to net-next/master: > >> > >> Applying: ARM: dts: AM33XX: Add memory resource to d_can node > >> error: patch failed: arch/arm/boot/dts/am33xx.dtsi:227 > >> error: arch/arm/boot/dts/am33xx.dtsi: patch does not apply > >> Patch failed at 0003 ARM: dts: AM33XX: Add memory resource to d_can node > >> > > > > Marc, > > > > Sorry about this DT changes are present in linux-omap. > > > > Could you please take the driver changes only and ACK on remaining will > > help. > > Will do - I'm currently looking at the driver. > Hi Marc, Please ACK this patch as well. Thanks AnilKumar ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: [PATCH 1/3] can: c_can: Add d_can raminit support
On Tue, Nov 20, 2012 at 16:20:41, Marc Kleine-Budde wrote:
> On 11/14/2012 07:08 PM, AnilKumar Ch wrote:
> > Add D_CAN raminit support to C_CAN driver to enable D_CAN RAM,
> > which holds all the message objects during transmission or
> > receiving of data. This initialization/de-initialization should
> > be done in synchronous with D_CAN clock.
> >
> > In case of AM335X-EVM (active user of D_CAN driver) message RAM is
> > controlled through control module register for both instances. So
> > control module register details is required to initialization or
> > de-initialization of message RAM according to instance number.
> >
> > Control module memory resource is obtained from D_CAN dt node and
> > instance number obtained from device tree aliases node.
> >
> > Signed-off-by: AnilKumar Ch
>
> Some nitpicks inline.
Thanks for the review.
>
> > ---
> > drivers/net/can/c_can/c_can.c | 12 +++
> > drivers/net/can/c_can/c_can.h |3 +++
> > drivers/net/can/c_can/c_can_platform.c | 34
> > +++-
> > 3 files changed, 48 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/can/c_can/c_can.c b/drivers/net/can/c_can/c_can.c
> > index e5180df..c15830c 100644
> > --- a/drivers/net/can/c_can/c_can.c
> > +++ b/drivers/net/can/c_can/c_can.c
> > @@ -233,6 +233,12 @@ static inline void c_can_pm_runtime_put_sync(const
> > struct c_can_priv *priv)
> > pm_runtime_put_sync(priv->device);
> > }
> >
> > +static inline void c_can_reset_ram(const struct c_can_priv *priv, bool
> > enable)
> > +{
> > + if (priv->ram_init)
> > + priv->ram_init(priv, enable);
> > +}
> > +
> > static inline int get_tx_next_msg_obj(const struct c_can_priv *priv)
> > {
> > return (priv->tx_next & C_CAN_NEXT_MSG_OBJ_MASK) +
> > @@ -1090,6 +1096,7 @@ static int c_can_open(struct net_device *dev)
> > struct c_can_priv *priv = netdev_priv(dev);
> >
> > c_can_pm_runtime_get_sync(priv);
> > + c_can_reset_ram(priv, true);
> >
> > /* open the can device */
> > err = open_candev(dev);
> > @@ -1118,6 +1125,7 @@ static int c_can_open(struct net_device *dev)
> > exit_irq_fail:
> > close_candev(dev);
> > exit_open_fail:
> > + c_can_reset_ram(priv, false);
> > c_can_pm_runtime_put_sync(priv);
> > return err;
> > }
> > @@ -1131,6 +1139,8 @@ static int c_can_close(struct net_device *dev)
> > c_can_stop(dev);
> > free_irq(dev->irq, dev);
> > close_candev(dev);
> > +
> > + c_can_reset_ram(priv, false);
> > c_can_pm_runtime_put_sync(priv);
> >
> > return 0;
> > @@ -1188,6 +1198,7 @@ int c_can_power_down(struct net_device *dev)
> >
> > c_can_stop(dev);
> >
> > + c_can_reset_ram(priv, false);
> > c_can_pm_runtime_put_sync(priv);
> >
> > return 0;
> > @@ -1206,6 +1217,7 @@ int c_can_power_up(struct net_device *dev)
> > WARN_ON(priv->type != BOSCH_D_CAN);
> >
> > c_can_pm_runtime_get_sync(priv);
> > + c_can_reset_ram(priv, true);
> >
> > /* Clear PDR and INIT bits */
> > val = priv->read_reg(priv, C_CAN_CTRL_EX_REG);
> > diff --git a/drivers/net/can/c_can/c_can.h b/drivers/net/can/c_can/c_can.h
> > index e5ed41d..419de5c 100644
> > --- a/drivers/net/can/c_can/c_can.h
> > +++ b/drivers/net/can/c_can/c_can.h
> > @@ -169,6 +169,9 @@ struct c_can_priv {
> > void *priv; /* for board-specific data */
> > u16 irqstatus;
> > enum c_can_dev_id type;
> > + u32 __iomem *raminit_ctrlreg;
> > + unsigned int instance;
> > + void (*ram_init) (const struct c_can_priv *priv, bool enable);
> > };
> >
> > struct net_device *alloc_c_can_dev(void);
> > diff --git a/drivers/net/can/c_can/c_can_platform.c
> > b/drivers/net/can/c_can/c_can_platform.c
> > index ee141613..2e61d69 100644
> > --- a/drivers/net/can/c_can/c_can_platform.c
> > +++ b/drivers/net/can/c_can/c_can_platform.c
> > @@ -38,6 +38,8 @@
> >
> > #include "c_can.h"
> >
> > +#define CAN_RAMINIT_START_MASK(i) (1 << (i))
> > +
> > /*
> > * 16-bit c_can registers can be arranged differently in the memory
> > * architecture of different implementations. For example: 16-bit
> > @@ -68,6 +70,24 @@ static void c_can_plat_write_reg_aligned_to_32bit(struct
> > c_can_priv *priv,
> > writew(val, priv->base + 2 * priv->regs[index]);
> > }
> >
> > +static void c_can_hw_raminit(const struct c_can_priv *priv, bool enable)
> > +{
> > + u32 val;
> > +
> > + if (!priv->raminit_ctrlreg || (priv->instance < 0))
> > + return;
>
> Can you move this sanity check to the probe function and only assign
> priv->ram_init if the above is true?
Sure, I will change
>
> > +
> > + val = readl(priv->raminit_ctrlreg);
> > + if (enable) {
> > + val &= ~CAN_RAMINIT_START_MASK(priv->instance);
> > + val |= CAN_RAMINIT_START_MASK(priv->instance);
> > + writel(val, priv->raminit_ctrlreg);
> > + } else {
> > + val &= ~CAN_RAMINIT_START_MASK(priv-
RE: [PATCH 3/3] ARM: dts: AM33XX: Add memory resource to d_can node
On Tue, Nov 20, 2012 at 15:43:04, Marc Kleine-Budde wrote: > On 11/14/2012 07:08 PM, AnilKumar Ch wrote: > > Add a new address space/memory resource to d_can device tree node. D_CAN > > RAM initialization is achieved through RAMINIT register which is part of > > AM33XX control module address space. D_CAN RAM init or de-init should be > > done by writing instance corresponding value to control module register. > > > > Till we have a separate control module driver to write to control module, > > d_can driver will handle the register writes to control module by itself. > > So a new address space to represent this control module register is added > > to d_can driver. > > > > Signed-off-by: AnilKumar Ch > > This does not apply to net-next/master: > > Applying: ARM: dts: AM33XX: Add memory resource to d_can node > error: patch failed: arch/arm/boot/dts/am33xx.dtsi:227 > error: arch/arm/boot/dts/am33xx.dtsi: patch does not apply > Patch failed at 0003 ARM: dts: AM33XX: Add memory resource to d_can node > Marc, Sorry about this DT changes are present in linux-omap. Could you please take the driver changes only and ACK on remaining will help. Tony/Benoit, Could you please take dt patches in this series to linux-omap? Thanks AnilKumar ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: [PATCH 0/3] can: Add D_CAN raminit support to am335x-evm
On Wed, Nov 14, 2012 at 23:38:22, AnilKumar, Chimata wrote: > This patch series adds d_can raminit support to c_can/d_can driver, > which is required to init/de-init D_CAN message RAM (holds message > objects). Added corresponding DT changes to get resource of RAMINIT > register and device instance. > > These patches were tested on AM335x-EVM along with pinctrl data addition > patch, which is not added to am335x-evm.dts (only supports CPLD profile#0) > because d_can1 is supported under CPLD profile#1. Hi Mark, Gentle remainder to this series Thanks AnilKumar > AnilKumar Ch (3): > can: c_can: Add d_can raminit support > ARM: dts: AM33XX: Add d_can instances to aliases > ARM: dts: AM33XX: Add memory resource to d_can node > > arch/arm/boot/dts/am33xx.dtsi |8 ++-- > drivers/net/can/c_can/c_can.c | 12 +++ > drivers/net/can/c_can/c_can.h |3 +++ > drivers/net/can/c_can/c_can_platform.c | 34 > +++- > 4 files changed, 54 insertions(+), 3 deletions(-) > > -- > 1.7.9.5 > > ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: [PATCH v2 1/4] mfd: tps65217: Set PMIC to shutdowm on PWR_EN toggle
On Mon, Nov 19, 2012 at 16:20:27, Peter Korsgaard wrote: > > "AnilKumar" == AnilKumar Ch writes: > > s/shutdowm/shutdown/ in the subject. I will change > > AnilKumar> From: Colin Foe-Parker > AnilKumar> Set tps65217 PMIC status to OFF if power enable toggle is > AnilKumar> supported. Also adds platform data flag, which should be > AnilKumar> passed from board init data. > > You're adding dt binding, not platform data. I will change > > AnilKumar> Signed-off-by: Colin Foe-Parker > AnilKumar> [anilku...@ti.com: move the additions to tps65217 MFD driver] > AnilKumar> Signed-off-by: AnilKumar Ch > AnilKumar> --- > AnilKumar> .../devicetree/bindings/regulator/tps65217.txt |4 > AnilKumar> drivers/mfd/tps65217.c | 12 > > AnilKumar> 2 files changed, 16 insertions(+) > > AnilKumar> diff --git > a/Documentation/devicetree/bindings/regulator/tps65217.txt > b/Documentation/devicetree/bindings/regulator/tps65217.txt > AnilKumar> index d316fb8..4f05d20 100644 > AnilKumar> --- a/Documentation/devicetree/bindings/regulator/tps65217.txt > AnilKumar> +++ b/Documentation/devicetree/bindings/regulator/tps65217.txt > AnilKumar> @@ -11,6 +11,9 @@ Required properties: > AnilKumar>using the standard binding for regulators found at > AnilKumar>Documentation/devicetree/bindings/regulator/regulator.txt. > > AnilKumar> +Optional properties: > AnilKumar> +- ti,pmic-shutdown-controller: Telling the PMIC to shutdown on > PWR_EN toggle. > > Ehh, I don't know the tps65217 particular well, but according to the > datasheet the REG_STATUS_OFF bit enables/disables the PB_IN pin, not > PWR_EN. It's also not only about powering down but also powering up > again. > > I don't feel this property name is very clear. Perhaps something about > power button or push button monitor as it is called in the datasheet? Here I am writing "TPS65217_STATUS_OFF" to status register, BIT(7). Description of this bit field: "OFF bit. Set this bit to 1 to enter OFF state when PWR_EN pin is pulled low. Bit is automatically reset to 0." PWR_EN pin pull-down is doing by RTC module. Regards AnilKumar ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: [PATCH v2 4/4] ARM: dts: AM33XX: Enable system power off control in am335x-bone
On Sat, Nov 17, 2012 at 00:40:04, Kevin Hilman wrote:
> AnilKumar Ch writes:
>
> > Enable system power off control for BeagleBone in am335x-bone.dts file
> > under rtc node. RTC is the incharge of controlling the system power.
> > This flag is used by the driver to hook up the pm_power_off system call.
> >
> > Signed-off-by: AnilKumar Ch
> > ---
> > arch/arm/boot/dts/am335x-bone.dts |4
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/arch/arm/boot/dts/am335x-bone.dts
> > b/arch/arm/boot/dts/am335x-bone.dts
> > index 1d55190..206c3eb 100644
> > --- a/arch/arm/boot/dts/am335x-bone.dts
> > +++ b/arch/arm/boot/dts/am335x-bone.dts
> > @@ -52,6 +52,10 @@
> > };
> >
> > };
> > +
> > + rtc@44e3e000 {
> > + ti,system-power-controller;
> > + };
> > };
>
> Also, I think this series is missing a patch that allows the RTC driver
> to be compiled on AM335x.
>
Hi Kevin,
Yes I missed; I will submit that as a separate patch.
Thanks
AnilKumar
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: [PATCH 2/4] rtc: OMAP: Add system pm_power_off to rtc driver
On Fri, Nov 16, 2012 at 11:43:36, AnilKumar, Chimata wrote:
> On Mon, Nov 12, 2012 at 15:17:48, AnilKumar, Chimata wrote:
> > On Tue, Nov 06, 2012 at 11:15:34, Bedia, Vaibhav wrote:
> > > On Mon, Nov 05, 2012 at 15:12:27, AnilKumar, Chimata wrote:
> > > [...]
> > > >
> > > > +#define SHUTDOWN_TIME_SEC 2
> > > > +#define SECS_IN_MIN60
> > > > +#define WAIT_AFTER (SECS_IN_MIN -
> > > > SHUTDOWN_TIME_SEC)
> > > > +#define WAIT_TIME_MS (SHUTDOWN_TIME_SEC * 1000)
> > > > +
> > > > static void __iomem*rtc_base;
> > > >
> > > [...]
> > > > +
> > > > + /* Wait few seconds instead of rollover */
> > > > + do {
> > > > + omap_rtc_read_time(NULL, &tm);
> > > > + if (WAIT_AFTER <= tm.tm_sec)
> > > > + mdelay(WAIT_TIME_MS);
> > > > + } while (WAIT_AFTER <= tm.tm_sec);
> > >
> > > This hardcoded wait for rollover doesn't look good. I see some
> > > helper functions in rtc-lib.c which probably could be used for
> > > converting the current time to elapsed seconds, add the delay and
> > > then convert it back to the time to be programmed in RTC without
> > > worrying about rollover. Why not use that?
> >
> > I am not aware of those APIs, can you point some?
>
> I have gone through rtc-lib.c, these are the API's I am seeing
> in the library
>
> 1. rtc_time_to_tm: Convert seconds since 01-01-1970 00:00:00 to
> Gregorian date
> 2. rtc_tm_to_time: Convert Gregorian date to seconds since
> 01-01-1970 00:00:00
>
> Steps I followed:-
>
> 1: unsigned long time;
> 2: omap_rtc_read_time(NULL, &tm);
> 3: rtc_tm_to_time(tm, &time);
> 4: pr_info("Time 1 %lu\n", time);
> 5: time += 2; /* (2sec) */
> 6: rtc_time_to_tm(time, tm);
> 7: rtc_tm_to_time(tm, &time); /* Only for printing time value */
> 8: pr_info("Time 2 %lu\n", time);
>
> With the above steps I am seeing completely two different time
> values at step4 and step8
>
Nevermind it working now.
Thanks
AnilKumar
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: [PATCH 1/4] mfd: tps65217: Set PMIC to shutdowm on PWR_EN toggle
On Wed, Nov 14, 2012 at 15:54:53, Mark Brown wrote: > On Wed, Nov 14, 2012 at 11:08:49AM +0100, Benoit Cousson wrote: > > > I was wondering that, because exposing a pin to control the whole PMIC > > low power mode seems to be something that should be generic enough to be > > handled by the regulator framework. > > Having something that's controlled by software is really not at all > generic - suspending a PMIC from a GPIO is generally tied in very > closely with the CPU power sequencing which means it's typically some > combination of very hard coded things that we can't control or part of > much wider control of sequencing. > > > In the current situation we do have a pwr_en pin that can be controlled > > by a GPIO or whatever signal from the SoC. > > That's very similar, at PMIC level, to the fixedregulator that allow a > > GPIO binding to enable it. > > > Don't you think that should deserve a support in the fmwk? > > I'm not seeing a coherent description of a feature here - what exactly > are you proposing that we do? When and how would this GPIO be set for > example? It would be better if these patches are going in like this till the framework exists. We can change/move this portion once the framework is defined for this kind. Thanks AnilKumar ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: [PATCH 2/4] rtc: OMAP: Add system pm_power_off to rtc driver
On Mon, Nov 12, 2012 at 15:17:48, AnilKumar, Chimata wrote:
> On Tue, Nov 06, 2012 at 11:15:34, Bedia, Vaibhav wrote:
> > On Mon, Nov 05, 2012 at 15:12:27, AnilKumar, Chimata wrote:
> > [...]
> > >
> > > +#define SHUTDOWN_TIME_SEC2
> > > +#define SECS_IN_MIN 60
> > > +#define WAIT_AFTER (SECS_IN_MIN -
> > > SHUTDOWN_TIME_SEC)
> > > +#define WAIT_TIME_MS (SHUTDOWN_TIME_SEC * 1000)
> > > +
> > > static void __iomem *rtc_base;
> > >
> > [...]
> > > +
> > > + /* Wait few seconds instead of rollover */
> > > + do {
> > > + omap_rtc_read_time(NULL, &tm);
> > > + if (WAIT_AFTER <= tm.tm_sec)
> > > + mdelay(WAIT_TIME_MS);
> > > + } while (WAIT_AFTER <= tm.tm_sec);
> >
> > This hardcoded wait for rollover doesn't look good. I see some
> > helper functions in rtc-lib.c which probably could be used for
> > converting the current time to elapsed seconds, add the delay and
> > then convert it back to the time to be programmed in RTC without
> > worrying about rollover. Why not use that?
>
> I am not aware of those APIs, can you point some?
I have gone through rtc-lib.c, these are the API's I am seeing
in the library
1. rtc_time_to_tm: Convert seconds since 01-01-1970 00:00:00 to
Gregorian date
2. rtc_tm_to_time: Convert Gregorian date to seconds since
01-01-1970 00:00:00
Steps I followed:-
1: unsigned long time;
2: omap_rtc_read_time(NULL, &tm);
3: rtc_tm_to_time(tm, &time);
4: pr_info("Time 1 %lu\n", time);
5: time += 2; /* (2sec) */
6: rtc_time_to_tm(time, tm);
7: rtc_tm_to_time(tm, &time); /* Only for printing time value */
8: pr_info("Time 2 %lu\n", time);
With the above steps I am seeing completely two different time
values at step4 and step8
Thanks
AnilKumar
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: [PATCH v6] Input: matrix-keypad - Add device tree support
On Thu, Nov 15, 2012 at 19:26:24, Rob Herring wrote: > On 11/15/2012 04:42 AM, AnilKumar Ch wrote: > > Add device tree support to matrix keypad driver and usage details > > are added to device tree documentation. Also modified the driver > > according to recent update of matrix_keypad_build_keymap(), which > > automatically allocate memory for keymap. > > > > Signed-off-by: AnilKumar Ch > > Signed-off-by: Dmitry Torokhov > > --- > > This patch was tested on AM335x-EVM and based on linux-next, cleanly > > applies on top of "input/next" > > > > Changes from v5: > > - Updated based on Dmitry's patch. > > * Modified the driver acc. to matrix_keypad_build_keymap() > > recent update. > > * Combined row_gpios & col_gpios devm_kzalloc's to single > > devm_kzalloc. > > * Added proper return values for different failures in > > matrix_keypad_parse_dt(). > > > > Changes from v4: > > - Removed clustered-irq support through DT. This should be > > added if any platform requires in the future. > > > > Changes from v3: > > - Incorporated Stephen Warren's review comments on v3 > > * Added description to clustered-irq-flags binding > > > > Changes from v2: > > - Incorporated Stephen Warren's review comments on v2 > > * Renamed the bindings file to gpio-matrix-keypad.txt > > * Added description to clustered-irq binding > > > > Changes from v1: > > - Incorporated Rob's review comments on v1 > > * Changed matix-keypad compatible to gpio-matrix-keypad > > * Removed unnecessary props (num of gpios) > > * Used of_match_ptr() > > - Removed first patch based on Dmitry's comments on v1 > > > > drivers/input/keyboard/matrix_keypad.c | 119 > > ++-- > > What happened to the binding doc? Rebase mistake, right away I am resending the patch. Thanks AnilKumar ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: [PATCH v5] Input: matrix-keypad - Add device tree support
On Thu, Nov 15, 2012 at 13:52:30, Dmitry Torokhov wrote:
> Hi AnilKumar,
>
> On Thu, Nov 15, 2012 at 07:07:33AM +, AnilKumar, Chimata wrote:
> > On Sun, Nov 11, 2012 at 08:05:18, Rob Herring wrote:
> > > On 11/10/2012 03:40 PM, AnilKumar Ch wrote:
> > > > Add device tree support to matrix keypad driver and usage details
> > > > are added to device tree documentation. Driver was tested on AM335x
> > > > EVM.
> > > >
> > > > Signed-off-by: AnilKumar Ch
> > >
> > > Acked-by: Rob Herring
> >
> > Hi Dmitry,
> >
> > If there are no further comments on this patch, could you please
> > take this in?
>
> Actually, could you please try the version below? Note that it needs
> changes to matrix_keypad_build_keymap() found in 'next' branch of my
> tree (to allocate keymap if caller did not supply it).
>
Hi Dmitry,
Thanks for the patch. Minor fix should be required.
>
>
> Input: matrix-keypad - add device tree support
>
> From: AnilKumar Ch
>
> Add device tree support to matrix keypad driver and usage details
> are added to device tree documentation. Driver was tested on AM335x
> EVM.
>
> Signed-off-by: AnilKumar Ch
> Signed-off-by: Dmitry Torokhov
> ---
> .../bindings/input/gpio-matrix-keypad.txt | 46
> drivers/input/keyboard/matrix_keypad.c | 119
>
> 2 files changed, 143 insertions(+), 22 deletions(-)
> create mode 100644
> Documentation/devicetree/bindings/input/gpio-matrix-keypad.txt
>
> diff --git a/Documentation/devicetree/bindings/input/gpio-matrix-keypad.txt
> b/Documentation/devicetree/bindings/input/gpio-matrix-keypad.txt
> new file mode 100644
> index 000..ead641c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/gpio-matrix-keypad.txt
> @@ -0,0 +1,46 @@
> +* GPIO driven matrix keypad device tree bindings
> +
> +GPIO driven matrix keypad is used to interface a SoC with a matrix keypad.
> +The matrix keypad supports multiple row and column lines, a key can be
> +placed at each intersection of a unique row and a unique column. The matrix
> +keypad can sense a key-press and key-release by means of GPIO lines and
> +report the event using GPIO interrupts to the cpu.
> +
> +Required Properties:
> +- compatible:Should be "gpio-matrix-keypad"
> +- row-gpios: List of gpios used as row lines. The gpio specifier
> + for this property depends on the gpio controller to
> + which these row lines are connected.
> +- col-gpios: List of gpios used as column lines. The gpio specifier
> + for this property depends on the gpio controller to
> + which these column lines are connected.
> +- linux,keymap: The definition can be found at
> + bindings/input/matrix-keymap.txt
> +
> +Optional Properties:
> +- linux,no-autorepeat: do no enable autorepeat feature.
> +- linux,wakeup: use any event on keypad as wakeup event.
> +- debounce-delay-ms: debounce interval in milliseconds
> +- col-scan-delay-us: delay, measured in microseconds, that is needed
> + before we can scan keypad after activating column gpio
> +
> +Example:
> + matrix-keypad {
> + compatible = "gpio-matrix-keypad";
> + debounce-delay-ms = <5>;
> + col-scan-delay-us = <2>;
> +
> + row-gpios = <&gpio2 25 0
> + &gpio2 26 0
> + &gpio2 27 0>;
> +
> + col-gpios = <&gpio2 21 0
> + &gpio2 22 0>;
> +
> + linux,keymap = <0x008B
> + 0x019E
> + 0x0269
> + 0x0001006A
> + 0x0101001C
> + 0x0201006C>;
> + };
> diff --git a/drivers/input/keyboard/matrix_keypad.c
> b/drivers/input/keyboard/matrix_keypad.c
> index 18b7237..ebf6e8f 100644
> --- a/drivers/input/keyboard/matrix_keypad.c
> +++ b/drivers/input/keyboard/matrix_keypad.c
> @@ -23,6 +23,9 @@
> #include
> #include
> #include
> +#include
> +#include
> +#include
>
> struct matrix_keypad {
> const struct matrix_keypad_platform_data *pdata;
> @@ -37,8 +40,6 @@ struct matrix_keypad {
> bool scan_pending;
> bool stopped;
> bool gpio_all_disabled;
> -
> - unsigned short keycodes[];
> };
>
> /*
>
RE: [PATCH v5] Input: matrix-keypad - Add device tree support
On Sun, Nov 11, 2012 at 08:05:18, Rob Herring wrote:
> On 11/10/2012 03:40 PM, AnilKumar Ch wrote:
> > Add device tree support to matrix keypad driver and usage details
> > are added to device tree documentation. Driver was tested on AM335x
> > EVM.
> >
> > Signed-off-by: AnilKumar Ch
>
> Acked-by: Rob Herring
Hi Dmitry,
If there are no further comments on this patch, could you please
take this in?
Thanks
AnilKumar
>
> > ---
> > This patch was tested on AM335x-EVM and based on linux-next, cleanly
> > applies on top of "input/next"
> >
> > Changes from v4:
> > - Removed clustered-irq support through DT. This should be
> > added if any platform requires in the future.
> >
> > Changes from v3:
> > - Incorporated Stephen Warren's review comments on v3
> > * Added description to clustered-irq-flags binding
> >
> > Changes from v2:
> > - Incorporated Stephen Warren's review comments on v2
> > * Renamed the bindings file to gpio-matrix-keypad.txt
> > * Added description to clustered-irq binding
> >
> > Changes from v1:
> > - Incorporated Rob's review comments on v1
> > * Changed matix-keypad compatible to gpio-matrix-keypad
> > * Removed unnecessary props (num of gpios)
> > * Used of_match_ptr()
> > - Removed first patch based on Dmitry's comments on v1
> >
> > .../bindings/input/gpio-matrix-keypad.txt | 46 +
> > drivers/input/keyboard/matrix_keypad.c | 104
> > +++-
> > 2 files changed, 149 insertions(+), 1 deletion(-)
> > create mode 100644
> > Documentation/devicetree/bindings/input/gpio-matrix-keypad.txt
> >
> > diff --git a/Documentation/devicetree/bindings/input/gpio-matrix-keypad.txt
> > b/Documentation/devicetree/bindings/input/gpio-matrix-keypad.txt
> > new file mode 100644
> > index 000..ead641c
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/input/gpio-matrix-keypad.txt
> > @@ -0,0 +1,46 @@
> > +* GPIO driven matrix keypad device tree bindings
> > +
> > +GPIO driven matrix keypad is used to interface a SoC with a matrix keypad.
> > +The matrix keypad supports multiple row and column lines, a key can be
> > +placed at each intersection of a unique row and a unique column. The matrix
> > +keypad can sense a key-press and key-release by means of GPIO lines and
> > +report the event using GPIO interrupts to the cpu.
> > +
> > +Required Properties:
> > +- compatible: Should be "gpio-matrix-keypad"
> > +- row-gpios: List of gpios used as row lines. The gpio
> > specifier
> > + for this property depends on the gpio controller to
> > + which these row lines are connected.
> > +- col-gpios: List of gpios used as column lines. The gpio
> > specifier
> > + for this property depends on the gpio controller to
> > + which these column lines are connected.
> > +- linux,keymap:The definition can be found at
> > + bindings/input/matrix-keymap.txt
> > +
> > +Optional Properties:
> > +- linux,no-autorepeat: do no enable autorepeat feature.
> > +- linux,wakeup:use any event on keypad as wakeup event.
> > +- debounce-delay-ms: debounce interval in milliseconds
> > +- col-scan-delay-us: delay, measured in microseconds, that is needed
> > + before we can scan keypad after activating column gpio
> > +
> > +Example:
> > + matrix-keypad {
> > + compatible = "gpio-matrix-keypad";
> > + debounce-delay-ms = <5>;
> > + col-scan-delay-us = <2>;
> > +
> > + row-gpios = <&gpio2 25 0
> > +&gpio2 26 0
> > +&gpio2 27 0>;
> > +
> > + col-gpios = <&gpio2 21 0
> > +&gpio2 22 0>;
> > +
> > + linux,keymap = <0x008B
> > + 0x019E
> > + 0x0269
> > + 0x0001006A
> > + 0x0101001C
> > + 0x0201006C>;
> > + };
> > diff --git a/drivers/input/keyboard/matrix_keypad.c
> > b/drivers/input/keyboard/matrix_keypad.c
> > index 18b7237..3ba8cfb 100644
> > --- a/drivers/input/keyboard/matrix_keypad.c
> > +++ b/drivers/input/keyboard/matrix_keypad.c
> > @@ -23,6 +23,9 @@
> > #include
> > #include
> > #include
> > +#include
> > +#include
> > +#include
> >
> > struct matrix_keypad {
> > const struct matrix_keypad_platform_data *pdata;
> > @@ -394,6 +397,93 @@ static void matrix_keypad_free_gpio(struct
> > matrix_keypad *keypad)
> > gpio_free(pdata->col_gpios[i]);
> > }
> >
> > +#ifdef CONFIG_OF
> > +static
> > +struct matrix_keypad_platform_data *matrix_keypad_parse_dt(struct device
> > *dev)
> > +{
> > + struct matrix_keypad_platform_data *pdata;
> > + struct matrix_keymap_data *keymap_data;
> > + struct device_node *
RE: [PATCH 1/4] mfd: tps65217: Set PMIC to shutdowm on PWR_EN toggle
On Wed, Nov 14, 2012 at 11:51:19, Mark Brown wrote: > On Wed, Nov 14, 2012 at 06:11:45AM +0000, AnilKumar, Chimata wrote: > > > From these two threads we can infer that this is handled in power_off > > sequence only. And this is feature of PMIC to go to shutdown mode nothing > > to be fixed in silicon. PWR_EN line can be connected to any of these like > > PRCM control or GPIO or some other instead of RTC(in this case). > > OK, but why are you telling me this? What are you looking for me to do > with this information? > Mark, Earlier you have a comment on this thread, I am adding my comments on top of it. Sorry if I am in wrong direction. Thanks AnilKumar ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: [PATCH 1/4] mfd: tps65217: Set PMIC to shutdowm on PWR_EN toggle
On Wed, Nov 14, 2012 at 10:40:18, AnilKumar, Chimata wrote: > On Wed, Nov 14, 2012 at 07:53:42, Mark Brown wrote: > > On Mon, Nov 05, 2012 at 05:59:36PM +0100, Benoit Cousson wrote: > > > On 11/05/2012 10:42 AM, AnilKumar Ch wrote: > > > > > > +Optional properties: > > > > +- ti,pmic-shutdown-controller: Telling the PMIC to shutdown on PWR_EN > > > > toggle. > > > > > That sounds like a generic functionality to me. Don't we have some more > > > generic way to handle that? > > > > > If not, that should probably not be a TI only attribute. > > > > It's pretty unusual to have this configurable as a single thing rather > > than as part of flexible power sequencing or something that's just fixed > > in silicon. > > Mark, >From these two threads we can infer that this is handled in power_off sequence only. And this is feature of PMIC to go to shutdown mode nothing to be fixed in silicon. PWR_EN line can be connected to any of these like PRCM control or GPIO or some other instead of RTC(in this case). Thanks AnilKumar > > "[PATCH 2/4] rtc: OMAP: Add system pm_power_off to rtc driver" thread > have the details of how PMIC is connected to RTC module of SoC. > > As part of the power_off sequence we have > 1. To write STATUS_OFF in TPS65217 PMIC. If we do so then PMIC will > go to shutdown if PWR_EN is pulled-down. (This patch doing this) > 2. To pull down the PWR_EN signal we have to set PMIC_PWR_EN in RTC > module and trigger ALARM2 event. (This piece of code in 2/4 patch). > > Thanks > AnilKumar > ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: [PATCH 2/4] rtc: OMAP: Add system pm_power_off to rtc driver
On Wed, Nov 14, 2012 at 10:31:42, AnilKumar, Chimata wrote:
> +Mark
>
> On Mon, Nov 12, 2012 at 15:17:13, AnilKumar, Chimata wrote:
> > On Tue, Nov 06, 2012 at 22:26:54, Cousson, Benoit wrote:
> > > Hi Anil,
> > >
> > > On 11/06/2012 06:07 AM, AnilKumar, Chimata wrote:
> > > > On Mon, Nov 05, 2012 at 22:13:25, Cousson, Benoit wrote:
> > > >> Hi Anil / Colin,
> > > >>
> > > >> On 11/05/2012 10:42 AM, AnilKumar Ch wrote:
> > > >>> From: Colin Foe-Parker
> > > >>>
> > > >>> Add system power off control to rtc driver which is the in-charge
> > > >>> of controlling the BeagleBone system power. The power_off routine
> > > >>> can be hooked up to "pm_power_off" system call.
> > > >>>
> > > >>> System power off sequence:-
> > > >>> * Set PMIC STATUS_OFF when PMIC_POWER_EN is pulled low
> > > >>> * Enable PMIC_POWER_EN in rtc module
> > > >>> * Set rtc ALARM2 time
> > > >>> * Enable ALARM2 interrupt
> > > >>>
> > > >>> Added while (1); after the above steps to make sure that no other
> > > >>> process acquire cpu. Otherwise we might see an unexpected behaviour
> > > >>> because we are shutting down all the power rails of SoC except RTC.
> > > >>>
> > > >>> Signed-off-by: Colin Foe-Parker
> > > >>> [anilku...@ti.com: move poweroff additions to rtc driver]
> > > >>> Signed-off-by: AnilKumar Ch
> > > >>> ---
> > > >>> Documentation/devicetree/bindings/rtc/rtc-omap.txt |5 ++
> > > >>> drivers/rtc/rtc-omap.c | 79
> > > >>> +++-
> > > >>> 2 files changed, 83 insertions(+), 1 deletion(-)
> > > >>>
> > > >>> diff --git a/Documentation/devicetree/bindings/rtc/rtc-omap.txt
> > > >>> b/Documentation/devicetree/bindings/rtc/rtc-omap.txt
> > > >>> index b47aa41..8d9f4f9 100644
> > > >>> --- a/Documentation/devicetree/bindings/rtc/rtc-omap.txt
> > > >>> +++ b/Documentation/devicetree/bindings/rtc/rtc-omap.txt
> > > >>> @@ -6,6 +6,10 @@ Required properties:
> > > >>> - interrupts: rtc timer, alarm interrupts in order
> > > >>> - interrupt-parent: phandle for the interrupt controller
> > > >>>
> > > >>> +Optional properties:
> > > >>> +- ti,system-power-controller: Telling whether or not rtc is
> > > >>> controlling
> > > >>> + the system power.
> > > >>
> > > >> I don't know how it is connected at board level, but I'm not sure the
> > > >> binding is the proper one.
> > > >
> > > > Hi Benoit,
> > > >
> > > > | __ ___ |
> > > > | | || | |
> > > > | |RTC || | |
> > > > | |PMIC | Line | | |
> > > > | |PWR_EN|===>|PWR_EN | |
> > > > | |__||___| |
> > > > | AM335x SoC TPS65217 |
> > > > ||
> > > > ||
> > > > BeagleBone
> > > >
> > > > This is how RTC PMIC_PWR_EN is connected to PWR_EN of TPS65217 PMIC.
> > > > Only when
> > > > RTC pull low in PMIC_PWR_EN then PMIC will go to power off state
> > > > provided TPS65217
> > > > status should be changed to STATUS_OFF.
> > > >
> > > > ALARM2 event should be trigger to configure PMIC_PWR_EN properly then
> > > > the "Line"
> > > > driven low so that PMIC will go to shutdown mode.
Mark,
Details regarding how PMIC PWR_EN is connected to RTC module
Thanks
AnilKumar
> > >
> > > Thanks for the nice diagram :-)
> >
> > I missed this mail thread so delayed in response
> >
> > >
> > > I'm wondering if we cannot abuse the gpio binding to describe that
> > > connection instead of creating two custom attributes (PMIC + RTC).
> > >
> > > Ideally we should do that without having to change the RTC to use the
> > > gpiolib at all.
> > >
> > >
> > > rtc: rtc@44e3e000 {
> > > compatible = "ti,da830-rtc";
> > > reg = <0x44e3e000 0x1000>;
> > > interrupts = <75, 76>;
> > > ti,hwmods = "rtc";
> > >
> > > /* expose the PWR_EN functionality of this RTC*/
> > > gpio-controller;
> > > #gpio-cells = <0>; /* assuming we can use 0 ??? */
> > > };
> > >
> > > ...
> > >
> > > tps: tps@24 {
> > > compatible = "ti,tps65217";
> > > /*
> > > * Enable the power enable feature from
> > > * the input line if that attribute is there.
> > > */
> > > gpio-power-en = <&rtc>; /* PWR_EN */
> > >
> > > ...
> > > }
> > >
> > > Any thought?
> >
> > No, these two are independent controllers. PMIC can go to power
> > off mode if we pull PWR_EN to low. We can pull down that line
> > by any means like PRCM or GPIO or some other. So these two flags
> > should be independent from each other.
> >
> > Thanks
> > AnilKumar
> >
>
>
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: [PATCH 1/4] mfd: tps65217: Set PMIC to shutdowm on PWR_EN toggle
On Wed, Nov 14, 2012 at 07:53:42, Mark Brown wrote: > On Mon, Nov 05, 2012 at 05:59:36PM +0100, Benoit Cousson wrote: > > On 11/05/2012 10:42 AM, AnilKumar Ch wrote: > > > > +Optional properties: > > > +- ti,pmic-shutdown-controller: Telling the PMIC to shutdown on PWR_EN > > > toggle. > > > That sounds like a generic functionality to me. Don't we have some more > > generic way to handle that? > > > If not, that should probably not be a TI only attribute. > > It's pretty unusual to have this configurable as a single thing rather > than as part of flexible power sequencing or something that's just fixed > in silicon. > "[PATCH 2/4] rtc: OMAP: Add system pm_power_off to rtc driver" thread have the details of how PMIC is connected to RTC module of SoC. As part of the power_off sequence we have 1. To write STATUS_OFF in TPS65217 PMIC. If we do so then PMIC will go to shutdown if PWR_EN is pulled-down. (This patch doing this) 2. To pull down the PWR_EN signal we have to set PMIC_PWR_EN in RTC module and trigger ALARM2 event. (This piece of code in 2/4 patch). Thanks AnilKumar ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: [PATCH 2/4] rtc: OMAP: Add system pm_power_off to rtc driver
+Mark
On Mon, Nov 12, 2012 at 15:17:13, AnilKumar, Chimata wrote:
> On Tue, Nov 06, 2012 at 22:26:54, Cousson, Benoit wrote:
> > Hi Anil,
> >
> > On 11/06/2012 06:07 AM, AnilKumar, Chimata wrote:
> > > On Mon, Nov 05, 2012 at 22:13:25, Cousson, Benoit wrote:
> > >> Hi Anil / Colin,
> > >>
> > >> On 11/05/2012 10:42 AM, AnilKumar Ch wrote:
> > >>> From: Colin Foe-Parker
> > >>>
> > >>> Add system power off control to rtc driver which is the in-charge
> > >>> of controlling the BeagleBone system power. The power_off routine
> > >>> can be hooked up to "pm_power_off" system call.
> > >>>
> > >>> System power off sequence:-
> > >>> * Set PMIC STATUS_OFF when PMIC_POWER_EN is pulled low
> > >>> * Enable PMIC_POWER_EN in rtc module
> > >>> * Set rtc ALARM2 time
> > >>> * Enable ALARM2 interrupt
> > >>>
> > >>> Added while (1); after the above steps to make sure that no other
> > >>> process acquire cpu. Otherwise we might see an unexpected behaviour
> > >>> because we are shutting down all the power rails of SoC except RTC.
> > >>>
> > >>> Signed-off-by: Colin Foe-Parker
> > >>> [anilku...@ti.com: move poweroff additions to rtc driver]
> > >>> Signed-off-by: AnilKumar Ch
> > >>> ---
> > >>> Documentation/devicetree/bindings/rtc/rtc-omap.txt |5 ++
> > >>> drivers/rtc/rtc-omap.c | 79
> > >>> +++-
> > >>> 2 files changed, 83 insertions(+), 1 deletion(-)
> > >>>
> > >>> diff --git a/Documentation/devicetree/bindings/rtc/rtc-omap.txt
> > >>> b/Documentation/devicetree/bindings/rtc/rtc-omap.txt
> > >>> index b47aa41..8d9f4f9 100644
> > >>> --- a/Documentation/devicetree/bindings/rtc/rtc-omap.txt
> > >>> +++ b/Documentation/devicetree/bindings/rtc/rtc-omap.txt
> > >>> @@ -6,6 +6,10 @@ Required properties:
> > >>> - interrupts: rtc timer, alarm interrupts in order
> > >>> - interrupt-parent: phandle for the interrupt controller
> > >>>
> > >>> +Optional properties:
> > >>> +- ti,system-power-controller: Telling whether or not rtc is controlling
> > >>> + the system power.
> > >>
> > >> I don't know how it is connected at board level, but I'm not sure the
> > >> binding is the proper one.
> > >
> > > Hi Benoit,
> > >
> > > | __ ___ |
> > > | | || | |
> > > | |RTC || | |
> > > | |PMIC | Line | | |
> > > | |PWR_EN|===>|PWR_EN | |
> > > | |__||___| |
> > > | AM335x SoC TPS65217 |
> > > ||
> > > ||
> > > BeagleBone
> > >
> > > This is how RTC PMIC_PWR_EN is connected to PWR_EN of TPS65217 PMIC. Only
> > > when
> > > RTC pull low in PMIC_PWR_EN then PMIC will go to power off state provided
> > > TPS65217
> > > status should be changed to STATUS_OFF.
> > >
> > > ALARM2 event should be trigger to configure PMIC_PWR_EN properly then the
> > > "Line"
> > > driven low so that PMIC will go to shutdown mode.
> >
> > Thanks for the nice diagram :-)
>
> I missed this mail thread so delayed in response
>
> >
> > I'm wondering if we cannot abuse the gpio binding to describe that
> > connection instead of creating two custom attributes (PMIC + RTC).
> >
> > Ideally we should do that without having to change the RTC to use the
> > gpiolib at all.
> >
> >
> > rtc: rtc@44e3e000 {
> > compatible = "ti,da830-rtc";
> > reg = <0x44e3e000 0x1000>;
> > interrupts = <75, 76>;
> > ti,hwmods = "rtc";
> >
> > /* expose the PWR_EN functionality of this RTC*/
> > gpio-controller;
> > #gpio-cells = <0>; /* assuming we can use 0 ??? */
> > };
> >
> > ...
> >
> > tps: tps@24 {
> > compatible = "ti,tps65217";
> > /*
> > * Enable the power enable feature from
> > * the input line if that attribute is there.
> > */
> > gpio-power-en = <&rtc>; /* PWR_EN */
> >
> > ...
> > }
> >
> > Any thought?
>
> No, these two are independent controllers. PMIC can go to power
> off mode if we pull PWR_EN to low. We can pull down that line
> by any means like PRCM or GPIO or some other. So these two flags
> should be independent from each other.
>
> Thanks
> AnilKumar
>
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: [PATCH 2/4] rtc: OMAP: Add system pm_power_off to rtc driver
On Tue, Nov 06, 2012 at 11:15:34, Bedia, Vaibhav wrote:
> On Mon, Nov 05, 2012 at 15:12:27, AnilKumar, Chimata wrote:
> [...]
> >
> > +#define SHUTDOWN_TIME_SEC 2
> > +#define SECS_IN_MIN60
> > +#define WAIT_AFTER (SECS_IN_MIN - SHUTDOWN_TIME_SEC)
> > +#define WAIT_TIME_MS (SHUTDOWN_TIME_SEC * 1000)
> > +
> > static void __iomem*rtc_base;
> >
> [...]
> > +
> > + /* Wait few seconds instead of rollover */
> > + do {
> > + omap_rtc_read_time(NULL, &tm);
> > + if (WAIT_AFTER <= tm.tm_sec)
> > + mdelay(WAIT_TIME_MS);
> > + } while (WAIT_AFTER <= tm.tm_sec);
>
> This hardcoded wait for rollover doesn't look good. I see some
> helper functions in rtc-lib.c which probably could be used for
> converting the current time to elapsed seconds, add the delay and
> then convert it back to the time to be programmed in RTC without
> worrying about rollover. Why not use that?
I am not aware of those APIs, can you point some?
>
> > +
> > + /* Add shutdown time to the current value */
> > + tm.tm_sec += SHUTDOWN_TIME_SEC;
> > +
> > + if (tm2bcd(&tm) < 0)
> > + return;
> > +
> > + pr_info("System will go to power_off state in approx. %d secs\n",
> > + SHUTDOWN_TIME_SEC);
> > +
> > + /* Set the ALARM2 time */
> > + rtc_write(tm.tm_sec, OMAP_RTC_ALARM2_SECONDS_REG);
> > + rtc_write(tm.tm_min, OMAP_RTC_ALARM2_MINUTES_REG);
> > + rtc_write(tm.tm_hour, OMAP_RTC_ALARM2_HOURS_REG);
> > + rtc_write(tm.tm_mday, OMAP_RTC_ALARM2_DAYS_REG);
> > + rtc_write(tm.tm_mon, OMAP_RTC_ALARM2_MONTHS_REG);
> > + rtc_write(tm.tm_year, OMAP_RTC_ALARM2_YEARS_REG);
> > +
> > + /* Enable alarm2 interrupt */
> > + val = readl(rtc_base + OMAP_RTC_INTERRUPTS_REG);
> > + writel(val | OMAP_RTC_INTERRUPTS_IT_ALARM2,
> > + rtc_base + OMAP_RTC_INTERRUPTS_REG);
> > +
>
> These registers are not present in older versions of the IP so how
> does that get handled?
I think, earlier this feature is not supported/not used.
>
> You also need to describe the connection between the ALARM2 and the
> power off logic in detail.
Sure, I will add.
Thanks
AnilKumar
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: [PATCH 2/4] rtc: OMAP: Add system pm_power_off to rtc driver
On Tue, Nov 06, 2012 at 22:26:54, Cousson, Benoit wrote:
> Hi Anil,
>
> On 11/06/2012 06:07 AM, AnilKumar, Chimata wrote:
> > On Mon, Nov 05, 2012 at 22:13:25, Cousson, Benoit wrote:
> >> Hi Anil / Colin,
> >>
> >> On 11/05/2012 10:42 AM, AnilKumar Ch wrote:
> >>> From: Colin Foe-Parker
> >>>
> >>> Add system power off control to rtc driver which is the in-charge
> >>> of controlling the BeagleBone system power. The power_off routine
> >>> can be hooked up to "pm_power_off" system call.
> >>>
> >>> System power off sequence:-
> >>> * Set PMIC STATUS_OFF when PMIC_POWER_EN is pulled low
> >>> * Enable PMIC_POWER_EN in rtc module
> >>> * Set rtc ALARM2 time
> >>> * Enable ALARM2 interrupt
> >>>
> >>> Added while (1); after the above steps to make sure that no other
> >>> process acquire cpu. Otherwise we might see an unexpected behaviour
> >>> because we are shutting down all the power rails of SoC except RTC.
> >>>
> >>> Signed-off-by: Colin Foe-Parker
> >>> [anilku...@ti.com: move poweroff additions to rtc driver]
> >>> Signed-off-by: AnilKumar Ch
> >>> ---
> >>> Documentation/devicetree/bindings/rtc/rtc-omap.txt |5 ++
> >>> drivers/rtc/rtc-omap.c | 79
> >>> +++-
> >>> 2 files changed, 83 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/rtc/rtc-omap.txt
> >>> b/Documentation/devicetree/bindings/rtc/rtc-omap.txt
> >>> index b47aa41..8d9f4f9 100644
> >>> --- a/Documentation/devicetree/bindings/rtc/rtc-omap.txt
> >>> +++ b/Documentation/devicetree/bindings/rtc/rtc-omap.txt
> >>> @@ -6,6 +6,10 @@ Required properties:
> >>> - interrupts: rtc timer, alarm interrupts in order
> >>> - interrupt-parent: phandle for the interrupt controller
> >>>
> >>> +Optional properties:
> >>> +- ti,system-power-controller: Telling whether or not rtc is controlling
> >>> + the system power.
> >>
> >> I don't know how it is connected at board level, but I'm not sure the
> >> binding is the proper one.
> >
> > Hi Benoit,
> >
> > | __ ___ |
> > | | || | |
> > | |RTC || | |
> > | |PMIC | Line | | |
> > | |PWR_EN|===>|PWR_EN | |
> > | |__||___| |
> > | AM335x SoC TPS65217 |
> > ||
> > ||
> > BeagleBone
> >
> > This is how RTC PMIC_PWR_EN is connected to PWR_EN of TPS65217 PMIC. Only
> > when
> > RTC pull low in PMIC_PWR_EN then PMIC will go to power off state provided
> > TPS65217
> > status should be changed to STATUS_OFF.
> >
> > ALARM2 event should be trigger to configure PMIC_PWR_EN properly then the
> > "Line"
> > driven low so that PMIC will go to shutdown mode.
>
> Thanks for the nice diagram :-)
I missed this mail thread so delayed in response
>
> I'm wondering if we cannot abuse the gpio binding to describe that
> connection instead of creating two custom attributes (PMIC + RTC).
>
> Ideally we should do that without having to change the RTC to use the
> gpiolib at all.
>
>
> rtc: rtc@44e3e000 {
> compatible = "ti,da830-rtc";
> reg = <0x44e3e000 0x1000>;
> interrupts = <75, 76>;
> ti,hwmods = "rtc";
>
> /* expose the PWR_EN functionality of this RTC*/
> gpio-controller;
> #gpio-cells = <0>; /* assuming we can use 0 ??? */
> };
>
> ...
>
> tps: tps@24 {
> compatible = "ti,tps65217";
> /*
> * Enable the power enable feature from
> * the input line if that attribute is there.
> */
> gpio-power-en = <&rtc>; /* PWR_EN */
>
> ...
> }
>
> Any thought?
No, these two are independent controllers. PMIC can go to power
off mode if we pull PWR_EN to low. We can pull down that line
by any means like PRCM or GPIO or some other. So these two flags
should be independent from each other.
Thanks
AnilKumar
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: [PATCH v4] Input: matrix-keypad - Add device tree support
On Thu, Nov 08, 2012 at 22:25:33, Stephen Warren wrote: > On 11/07/2012 02:32 AM, AnilKumar Ch wrote: > > Add device tree support to matrix keypad driver and usage details > > are added to device tree documentation. Driver was tested on AM335x > > EVM. > > > diff --git a/Documentation/devicetree/bindings/input/gpio-matrix-keypad.txt > > b/Documentation/devicetree/bindings/input/gpio-matrix-keypad.txt > > > +Optional Properties: > > > +- clustered-irq: have clustered irq number, that is needed if the irq > > + is a combined irq source for the whole matrix keypad. > > + This is useful if rows and columns of the keypad are > > + connected to a GPIO expander. > > +- clustered-irq-flags: clustered irq flags to specify the interrupt > > line > > + behavior among IRQF_TRIGGER_* > > I still don't understand why there's a need for a clustered-irq-flags > property; if those flags are the flags for an interrupt, why aren't the > flags part of the clustered-irq interrupt specifier, just like any other > interrupt in DT? Exactly, I agree with you on this. Honestly, I added clustered_xxx properties to DT considering the platform_data currently implemented in driver. And I do not have hardware to validate clustered_xxx execution flow. I looked at the commit which adds support for clustered_irq, Commit Description: "This one adds support of a combined irq source for the whole matrix keypad. This can be useful if all rows and columns of the keypad are e.g. connected to a GPIO expander, which only has one interrupt line for all events on every single GPIO." So I believe this was meant for matrix keypad interfaced over I2C expander, But I am not sure how it is being used. The hardware is connected in any of the following methods, the driver should supports the same. i) gpio interrupt can be used as keypad interrupt, connection is like this (MPU<===>IOEXP) |___| (gpio line) Here I expect driver should have gpio_to_irq implementation, but its missing from driver. So platform code must be doing it, but surprisingly I couldn't able to find any platform which uses this. ii) i2c interrupt can be used as keypad interrupt Here driver is not using threaded irq. As a reference I looked tca6416-keypad.c driver and it has right implementation, if (pdata->irq_is_gpio) chip->irqnum = gpio_to_irq(client->irq); else chip->irqnum = client->irq; As per my understanding matrix-keypad driver has to change accordingly to tca6416-keypad.c driver to handle clustered-irq. So I left with below options, 1. Lets only support normal GPIO based matrix key-pad, without clustered_irq support in DT. 2. Cleanup the driver to remove clustered_irq all together, considering the fact that we do not have any platform in kernel to use it. And once we get known platform in the future, we can again add it. I vote for option-1, what do you think? Thanks AnilKumar ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: [PATCH v4] Input: matrix-keypad - Add device tree support
On Wed, Nov 07, 2012 at 22:27:10, Stephen Warren wrote:
> On 11/07/2012 02:38 AM, AnilKumar, Chimata wrote:
> > On Wed, Nov 07, 2012 at 15:02:05, AnilKumar, Chimata wrote:
> >> Add device tree support to matrix keypad driver and usage details
> >> are added to device tree documentation. Driver was tested on AM335x
> >> EVM.
> >
> > +Stephen
> >
> > ACK from the reviewers (Rob Herring and Stephen Warren) of earlier
> > versions will help to get this in.
>
> I thought I already asked a question about the clustered IRQ properties,
> which don't make sense.
Hi Stephen,
In v4 I have added the details of clustered-irq properties
clustered-irq: have clustered irq number, that is needed if the irq
is a combined irq source for the whole matrix keypad. This is useful
if rows and columns of the keypad are connected to a GPIO expander.
clustered-irq-flags: clustered irq flags to specify the interrupt line
behavior among IRQF_TRIGGER_*.
These flags might vary depending on the hardware of the IO-expander
IRQ this flag is either IRQF_TRIGGER_RISING or IRQF_TRIGGER_FALLING or
IRQF_TRIGGER_HIGH or IRQF_TRIGGER_LOW or combinations.
The current matrix_keypad.c driver uses these parameters in this way
if (pdata->clustered_irq > 0) {
err = request_irq(pdata->clustered_irq,
matrix_keypad_interrupt,
pdata->clustered_irq_flags,
"matrix-keypad", keypad);
if (err) {
dev_err(&pdev->dev,
"Unable to acquire clustered interrupt\n");
goto err_free_rows;
}
}
In my v5 version I will remove these parameters and we can add if actual users
come into existence. My thought process was if somebody uses these, might
affect.
Thanks
AnilKumar
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: [PATCH v4] Input: matrix-keypad - Add device tree support
On Wed, Nov 07, 2012 at 15:02:05, AnilKumar, Chimata wrote:
> Add device tree support to matrix keypad driver and usage details
> are added to device tree documentation. Driver was tested on AM335x
> EVM.
+Stephen
ACK from the reviewers (Rob Herring and Stephen Warren) of earlier
versions will help to get this in.
Thanks
AnilKumar
>
> Signed-off-by: AnilKumar Ch
> ---
> This patch was tested on AM335x-EVM and based on linux-next, cleanly
> applies on top of input/next
>
> Changes from v3:
> - Incorporated Stephen Warren's review comments on v3
> * Added description to clustered-irq-flags binding
>
> Changes from v2:
> - Incorporated Stephen Warren's review comments on v2
> * Renamed the bindings file to gpio-matrix-keypad.txt
> * Added description to clustered-irq binding
>
> Changes from v1:
> - Incorporated Rob's review comments on v1
> * Changed matix-keypad compatible to gpio-matrix-keypad
> * Removed unnecessary props (num of gpios)
> * Used of_match_ptr()
> - Removed first patch based on Dmitry's comments on v1
>
> .../bindings/input/gpio-matrix-keypad.txt | 52 ++
> drivers/input/keyboard/matrix_keypad.c | 107
> +++-
> 2 files changed, 158 insertions(+), 1 deletion(-)
> create mode 100644
> Documentation/devicetree/bindings/input/gpio-matrix-keypad.txt
>
> diff --git a/Documentation/devicetree/bindings/input/gpio-matrix-keypad.txt
> b/Documentation/devicetree/bindings/input/gpio-matrix-keypad.txt
> new file mode 100644
> index 000..1acdfc4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/gpio-matrix-keypad.txt
> @@ -0,0 +1,52 @@
> +* GPIO driven matrix keypad device tree bindings
> +
> +GPIO driven matrix keypad is used to interface a SoC with a matrix keypad.
> +The matrix keypad supports multiple row and column lines, a key can be
> +placed at each intersection of a unique row and a unique column. The matrix
> +keypad can sense a key-press and key-release by means of GPIO lines and
> +report the event using GPIO interrupts to the cpu.
> +
> +Required Properties:
> +- compatible:Should be "gpio-matrix-keypad"
> +- row-gpios: List of gpios used as row lines. The gpio specifier
> + for this property depends on the gpio controller to
> + which these row lines are connected.
> +- col-gpios: List of gpios used as column lines. The gpio specifier
> + for this property depends on the gpio controller to
> + which these column lines are connected.
> +- linux,keymap: The definition can be found at
> + bindings/input/matrix-keymap.txt
> +
> +Optional Properties:
> +- linux,no-autorepeat: do no enable autorepeat feature.
> +- linux,wakeup: use any event on keypad as wakeup event.
> +- debounce-delay-ms: debounce interval in milliseconds
> +- col-scan-delay-us: delay, measured in microseconds, that is needed
> + before we can scan keypad after activating column gpio
> +- clustered-irq: have clustered irq number, that is needed if the irq
> + is a combined irq source for the whole matrix keypad.
> + This is useful if rows and columns of the keypad are
> + connected to a GPIO expander.
> +- clustered-irq-flags: clustered irq flags to specify the interrupt
> line
> + behaviour among IRQF_TRIGGER_*
> +
> +Example:
> + matrix-keypad {
> + compatible = "gpio-matrix-keypad";
> + debounce-delay-ms = <5>;
> + col-scan-delay-us = <2>;
> +
> + row-gpios = <&gpio2 25 0
> + &gpio2 26 0
> + &gpio2 27 0>;
> +
> + col-gpios = <&gpio2 21 0
> + &gpio2 22 0>;
> +
> + linux,keymap = <0x008B
> + 0x019E
> + 0x0269
> + 0x0001006A
> + 0x0101001C
> + 0x0201006C>;
> + };
> diff --git a/drivers/input/keyboard/matrix_keypad.c
> b/drivers/input/keyboard/matrix_keypad.c
> index 18b7237..960e9b05 100644
> --- a/drivers/input/keyboard/matrix_keypad.c
> +++ b/drivers/input/keyboard/matrix_keypad.c
> @@ -23,6 +23,9 @@
> #include
> #include
> #include
> +#include
> +#include
> +#include
>
> struct
RE: [PATCH RESEND 00/10] ARM: dts: AM33XX: Add device tree data
On Tue, Nov 06, 2012 at 20:26:48, Cousson, Benoit wrote: > Hi Anil, > > On 11/06/2012 02:48 PM, AnilKumar Ch wrote: > > Add device tree date for GPIO based various drivers matrix keypad, > > volume keys, push buttons and use leds accross three AM33XX devices > > viz EVM, BeagleBone and Starter Kit. > > > > To make it functional this series also adds pinctrl data for all > > the GPIOs used by various drivers. In this series only default state > > pinmux/conf settings are added because of sleep/idle state pinctrl > > values are not available. > > > > These patches are based on linux-omap-dt:for_3.8/dts_part2 tree and > > these were tested on am33xx devices according to added functionality. > > > > Change log: > > - Rebased on for_3.8/dts_part2 > > Thanks for the update. Applied in for_3.8/dts_part2. > > BTW, I've just noticed that am335x-evmsk is not built with make dtbs. The > target was missing from the arch/arm/boot/dts/Makefile. > > Please find below the patch to add it. Hi Benoit, Thanks for addition. Below change is missed from evmsk patch. Thanks AnilKumar > --- > From 6990451aca80a5107206688308302241f799057a Mon Sep 17 00:00:00 2001 > From: Benoit Cousson > Date: Tue, 6 Nov 2012 15:52:23 +0100 > Subject: [PATCH] ARM: dts: Makefile: Add the am335x-evmsk target in dtbs list > > The EVMSK was not built with the 'make dtbs' command. > Add the missing antry in the dts Makefile. > > Signed-off-by: Benoit Cousson > --- > arch/arm/boot/dts/Makefile |1 + > 1 files changed, 1 insertions(+), 0 deletions(-) > > diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile > index 634bd42..2458b69 100644 > --- a/arch/arm/boot/dts/Makefile > +++ b/arch/arm/boot/dts/Makefile > @@ -73,6 +73,7 @@ dtb-$(CONFIG_ARCH_OMAP2PLUS) += omap2420-h4.dtb \ > omap4-sdp.dtb \ > omap5-evm.dtb \ > am335x-evm.dtb \ > + am335x-evmsk.dtb \ > am335x-bone.dtb > dtb-$(CONFIG_ARCH_PRIMA2) += prima2-evb.dtb > dtb-$(CONFIG_ARCH_U8500) += snowball.dtb > -- > 1.7.0.4 > > ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: [PATCH 1/4] mfd: tps65217: Set PMIC to shutdowm on PWR_EN toggle
On Mon, Nov 05, 2012 at 22:29:36, Cousson, Benoit wrote: > + Mark > > On 11/05/2012 10:42 AM, AnilKumar Ch wrote: > > From: Colin Foe-Parker > > > > Set tps65217 PMIC status to OFF if power enable toggle is > > supported. Also adds platform data flag, which should be > > passed from board init data. > > > > Signed-off-by: Colin Foe-Parker > > [anilku...@ti.com: move the additions to tps65217 MFD driver] > > Signed-off-by: AnilKumar Ch > > --- > > .../devicetree/bindings/regulator/tps65217.txt |4 > > drivers/mfd/tps65217.c | 12 > > 2 files changed, 16 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/regulator/tps65217.txt > > b/Documentation/devicetree/bindings/regulator/tps65217.txt > > index d316fb8..4f05d20 100644 > > --- a/Documentation/devicetree/bindings/regulator/tps65217.txt > > +++ b/Documentation/devicetree/bindings/regulator/tps65217.txt > > @@ -11,6 +11,9 @@ Required properties: > >using the standard binding for regulators found at > >Documentation/devicetree/bindings/regulator/regulator.txt. > > > > +Optional properties: > > +- ti,pmic-shutdown-controller: Telling the PMIC to shutdown on PWR_EN > > toggle. > > That sounds like a generic functionality to me. Don't we have some more > generic way to handle that? But STATUS_OFF should be set only if Board supports it, otherwise this change doesn't make sense. > > If not, that should probably not be a TI only attribute. > > It looks like a GPIO like kind of interface at PMIC level. I agree this should be a generic parameter, but in some regulators this STATUS OFF control might not be available. This is in my mind while name this parameter. Thanks AnilKumar ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: [PATCH 2/4] rtc: OMAP: Add system pm_power_off to rtc driver
On Mon, Nov 05, 2012 at 22:13:25, Cousson, Benoit wrote:
> Hi Anil / Colin,
>
> On 11/05/2012 10:42 AM, AnilKumar Ch wrote:
> > From: Colin Foe-Parker
> >
> > Add system power off control to rtc driver which is the in-charge
> > of controlling the BeagleBone system power. The power_off routine
> > can be hooked up to "pm_power_off" system call.
> >
> > System power off sequence:-
> > * Set PMIC STATUS_OFF when PMIC_POWER_EN is pulled low
> > * Enable PMIC_POWER_EN in rtc module
> > * Set rtc ALARM2 time
> > * Enable ALARM2 interrupt
> >
> > Added while (1); after the above steps to make sure that no other
> > process acquire cpu. Otherwise we might see an unexpected behaviour
> > because we are shutting down all the power rails of SoC except RTC.
> >
> > Signed-off-by: Colin Foe-Parker
> > [anilku...@ti.com: move poweroff additions to rtc driver]
> > Signed-off-by: AnilKumar Ch
> > ---
> > Documentation/devicetree/bindings/rtc/rtc-omap.txt |5 ++
> > drivers/rtc/rtc-omap.c | 79
> > +++-
> > 2 files changed, 83 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/rtc/rtc-omap.txt
> > b/Documentation/devicetree/bindings/rtc/rtc-omap.txt
> > index b47aa41..8d9f4f9 100644
> > --- a/Documentation/devicetree/bindings/rtc/rtc-omap.txt
> > +++ b/Documentation/devicetree/bindings/rtc/rtc-omap.txt
> > @@ -6,6 +6,10 @@ Required properties:
> > - interrupts: rtc timer, alarm interrupts in order
> > - interrupt-parent: phandle for the interrupt controller
> >
> > +Optional properties:
> > +- ti,system-power-controller: Telling whether or not rtc is controlling
> > + the system power.
>
> I don't know how it is connected at board level, but I'm not sure the
> binding is the proper one.
Hi Benoit,
| __ ___ |
| | || | |
| |RTC || | |
| |PMIC | Line | | |
| |PWR_EN|===>|PWR_EN | |
| |__||___| |
| AM335x SoC TPS65217 |
||
||
BeagleBone
This is how RTC PMIC_PWR_EN is connected to PWR_EN of TPS65217 PMIC. Only when
RTC pull low in PMIC_PWR_EN then PMIC will go to power off state provided
TPS65217
status should be changed to STATUS_OFF.
ALARM2 event should be trigger to configure PMIC_PWR_EN properly then the "Line"
driven low so that PMIC will go to shutdown mode.
Thanks
AnilKumar
> It does not look super generic, and I'm wondering if we should not use
> instead some regulator binding to reflect the connection of the RTC to a
> regulator.
>
> But without the board / soc spec it is hard to tell :-(
>
> Regards,
> Benoit
>
>
> > +
> > Example:
> >
> > rtc@1c23000 {
> > @@ -14,4 +18,5 @@ rtc@1c23000 {
> > interrupts = <19
> > 19>;
> > interrupt-parent = <&intc>;
> > + ti,system-power-controller;
> > };
> > diff --git a/drivers/rtc/rtc-omap.c b/drivers/rtc/rtc-omap.c
> > index 6009714..2d90170 100644
> > --- a/drivers/rtc/rtc-omap.c
> > +++ b/drivers/rtc/rtc-omap.c
> > @@ -72,6 +72,14 @@
> > #define OMAP_RTC_KICK0_REG 0x6c
> > #define OMAP_RTC_KICK1_REG 0x70
> >
> > +#define OMAP_RTC_ALARM2_SECONDS_REG0x80
> > +#define OMAP_RTC_ALARM2_MINUTES_REG0x84
> > +#define OMAP_RTC_ALARM2_HOURS_REG 0x88
> > +#define OMAP_RTC_ALARM2_DAYS_REG 0x8c
> > +#define OMAP_RTC_ALARM2_MONTHS_REG 0x90
> > +#define OMAP_RTC_ALARM2_YEARS_REG 0x94
> > +#define OMAP_RTC_PMIC_REG 0x98
> > +
> > /* OMAP_RTC_CTRL_REG bit fields: */
> > #define OMAP_RTC_CTRL_SPLIT(1<<7)
> > #define OMAP_RTC_CTRL_DISABLE (1<<6)
> > @@ -93,15 +101,24 @@
> > #define OMAP_RTC_STATUS_BUSY(1<<0)
> >
> > /* OMAP_RTC_INTERRUPTS_REG bit fields: */
> > +#define OMAP_RTC_INTERRUPTS_IT_ALARM2 (1<<4)
> > #define OMAP_RTC_INTERRUPTS_IT_ALARM(1<<3)
> > #define OMAP_RTC_INTERRUPTS_IT_TIMER(1<<2)
> >
> > +/* OMAP_RTC_PMIC_REG bit fields: */
> > +#define OMAP_RTC_PMIC_POWER_EN_EN (1<<16)
> > +
> > /* OMAP_RTC_KICKER values */
> > #defineKICK0_VALUE 0x83e70b13
> > #defineKICK1_VALUE 0x95a4f1e0
> >
> > #defineOMAP_RTC_HAS_KICKER 0x1
> >
> > +#define SHUTDOWN_TIME_SEC 2
> > +#define SECS_IN_MIN60
> > +#define WAIT_AFTER (SECS_IN_MIN - SHUTDOWN_TIME_SEC)
> > +#define WAIT_TIME_MS (SHUTDOWN_TIME_SEC * 1000)
> > +
> > static void __iomem*rtc_base;
> >
> > #define rtc_read(addr) readb(rtc_base + (addr))
> > @@ -290,6 +307,58 @@ static int omap_rtc_set_alarm(struct device *dev,
> > struct rtc_wkalrm *alm)
> > return 0;
> > }
> >
> > +/*
> > + * rtc_power_off: Set the pmic power off sequence. The RTC generates
> > + * pmic_pwr_enable control, which
RE: [PATCH 00/10] ARM: dts: AM33XX: Add device tree data
On Mon, Nov 05, 2012 at 22:07:45, Cousson, Benoit wrote: > Hi Anil, > > On 11/05/2012 02:46 PM, AnilKumar, Chimata wrote: > > On Mon, Nov 05, 2012 at 16:15:40, AnilKumar, Chimata wrote: > >> Add device tree date for GPIO based various drivers matrix keypad, > >> volume keys, push buttons and use leds accross three AM33XX devices > >> viz EVM, BeagleBone and Starter Kit. > >> > >> To make it functional this series also adds pinctrl data for all > >> the GPIOs used by various drivers. In this series only default state > >> pinmux/conf settings are added because of sleep/idle state pinctrl > >> values are not available. > >> > >> These patches are based on linux-omap-dt:for_3.8/dts tree and these > > > > My bad, small correction in this statement, these patches apply cleanly > > on linux-omap-dt:for_3.8/dts tree with the below two patches. > > http://www.spinics.net/lists/arm-kernel/msg204872.html > > http://www.spinics.net/lists/arm-kernel/msg204873.html > > Mmm, this 2 patches do have dependency on some driver changes that might > not be accepted for 3.8. And in fact I do have some comment on the RTC > one :-) > Don't you prefer to change the order to allow me to pull these ones first? > Today I will resend the patches by removing this dependency. Thanks AnilKumar ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: [PATCH 00/10] ARM: dts: AM33XX: Add device tree data
On Mon, Nov 05, 2012 at 16:15:40, AnilKumar, Chimata wrote: > Add device tree date for GPIO based various drivers matrix keypad, > volume keys, push buttons and use leds accross three AM33XX devices > viz EVM, BeagleBone and Starter Kit. > > To make it functional this series also adds pinctrl data for all > the GPIOs used by various drivers. In this series only default state > pinmux/conf settings are added because of sleep/idle state pinctrl > values are not available. > > These patches are based on linux-omap-dt:for_3.8/dts tree and these My bad, small correction in this statement, these patches apply cleanly on linux-omap-dt:for_3.8/dts tree with the below two patches. http://www.spinics.net/lists/arm-kernel/msg204872.html http://www.spinics.net/lists/arm-kernel/msg204873.html Thanks AnilKumar > were tested on am33xx devices according to added functionality. > > AnilKumar Ch (10): > ARM: dts: AM33XX: Add pinmux configuration for matrix keypad to EVM > ARM: dts: AM33XX: Add matrix keypad device tree data to am335x-evm > ARM: dts: AM33XX: Add pinmux configuration for volume-keys to EVM > ARM: dts: AM33XX: Add volume-keys device tree data to am335x-evm > ARM: dts: AM33XX: Add pinmux configuration for user-leds to BONE > ARM: dts: AM33XX: Add user-leds device tree data to am335x-bone > ARM: dts: AM33XX: Add pinmux configuration for gpio-leds to EVMSK > ARM: dts: AM33XX: Add user-leds device tree data to am335x-evmsk > ARM: dts: AM33XX: Add pinmux configuration for gpio-keys to EVMSK > ARM: dts: AM33XX: Add push-buttons device tree data to am335x-evmsk > > arch/arm/boot/dts/am335x-bone.dts | 44 +++ > arch/arm/boot/dts/am335x-evm.dts | 63 +++ > arch/arm/boot/dts/am335x-evmsk.dts | 84 > > 3 files changed, 191 insertions(+) > > -- > 1.7.9.5 > > ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: [PATCH v3] Input: matrix-keypad - Add device tree support
+Luotao Fu On Sat, Nov 03, 2012 at 00:25:53, Stephen Warren wrote: > On 11/02/2012 05:03 AM, AnilKumar Ch wrote: > > Add device tree support to matrix keypad driver and usage details > > are added to device tree documentation. Driver was tested on AM335x > > EVM. > > > +++ b/Documentation/devicetree/bindings/input/gpio-matrix-keypad.txt > > > +Optional Properties: > > > +- clustered-irq: have clustered irq number, that is needed if the irq > > + is a combined irq source for the whole matix keypad. > > + This is useful if rows and columns of the keypad are > > + connected to a GPIO expander. > > +- clustered-irq-flags: clustered irq flags > > Shouldn't the GPIO expander itself be an IRQ controller too? What are > the clustered-irq-flags values? These are clustered-irq corresponding flags whether the IRQ is IRQF_TRIGGER_RISING (or) IRQF_TRIGGER_FALLING (or) both and etc. I have given the pointers based on this patch https://lkml.org/lkml/2010/5/27/45 I am not able to provide the further details because my hardware is different. Luotao Fu have added the clustered-irq support he might have some pointers here. Hi Luotao Fu, Do you have any comments here? Thanks AnilKumar ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: [PATCH v2] leds: leds-gpio: adopt pinctrl support
On Sun, Nov 04, 2012 at 23:07:44, Linus Walleij wrote: > On Tue, Oct 30, 2012 at 3:12 PM, AnilKumar, Chimata wrote: > > > I completely understood this named modes, I have added named > > modes like this in am335x-xxx.dts files > > I do not understand how the pinctrl-single dts files work actually, > so please get Tony to review this part. > > > I think "pinctrl-1" state will be used in driver > > suspend/resume calls. > > Hopefully, I think you should test the code and monitor the > system to make sure the right thing happens. I will test while adding "pinctrl-1" data to .dts files. > > > I have the pinmux/conf settings for default state but fully > > optimized pinmux/conf values in idle & suspend states are not > > available yet. > > You have defined a "sleep" state which is what should be used > for suspend? Or do you mean that you do have a state but > you're just not defining it to the most optimal setting yet? Yes, sleep state is used for suspend. Regarding to this suspend state fully optimized pinmux/conf values are not available. Thanks AnilKumar ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: [PATCH v2] Input: matrix-keypad - Add device tree support
On Wed, Oct 31, 2012 at 21:00:59, Stephen Warren wrote: > On 10/31/2012 05:54 AM, AnilKumar Ch wrote: > > Add device tree support to matrix keypad driver and usage details > > are added to device tree documentation. Driver was tested on AM335x > > EVM. > > > diff --git a/Documentation/devicetree/bindings/input/matrix-keypad.txt > > b/Documentation/devicetree/bindings/input/matrix-keypad.txt > > new file mode 100644 > > > +- compatible: Should be "gpio-matix-keypad" > > There's a typo there. > > Given that compatible value, shouldn't the file be named > gpio-matrix-keypad.txt then? Stephen, Thanks for the comments. I agree and I have same thing in my mind but the driver name is matrix-keypad.c. And I think we have to change the driver file names as well. (drivers/../matrix_keypad.c, linux/input/ matrix_keypad.h). > > It seems like the property linux,keymap should be mentioned here too, > with a note to read matrix-keymap.txt for the definition. I will add a point to specify "details of linux,keymap are available at matrix-keymap.txt" > > > +Optional Properties: > > > +- clustered-irq: have clustered irq number > > +- clustered-irq-flags: have clustered irq flags > > Explaining what those mean would be useful. Sure. Thanks AnilKumar ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: [PATCH v2] leds: leds-gpio: adopt pinctrl support
On Wed, Oct 03, 2012 at 18:06:10, Linus Walleij wrote:
> On Wed, Oct 3, 2012 at 12:52 PM, AnilKumar, Chimata wrote:
> > On Tue, Oct 02, 2012 at 01:29:37, Linus Walleij wrote:
>
> >> This is what we're doing for ux500 and should be a good model.
> >
> > I have looked into this, but not seen any named modes.
>
> OK maybe it's not easy to find. If you look into:
> arch/arm/mach-ux500/board-mop500-pins.c
> you find our work in progress. Note that this is not (yet)
> using device tree. (We want to migrate all our pinctrl
> stuff first, then do DT.)
>
> So for example this macro:
>
> #define DB8500_PIN(pin,conf,dev) \
> PIN_MAP_CONFIGS_PIN_DEFAULT(dev, "pinctrl-db8500", pin, conf)
>
> Will define a config for the "default" mode for a certain
> pin.
>
> This:
>
> #define DB8500_PIN_SLEEP(pin, conf, dev) \
> PIN_MAP_CONFIGS_PIN(dev, PINCTRL_STATE_SLEEP, "pinctrl-db8500", \
> pin, conf)
>
> Will mutatis mutandis define a "sleep" mode for a pin.
>
> Sorry for the macros. We'll get rid of them in the DT.
> (Now that Stephen has patched in preprocessing it will
> even look good.)
>
Hi Linus Walleij/Tony,
I completely understood this named modes, I have added named
modes like this in am335x-xxx.dts files
am33xx_pinmux: pinmux@44e10800 {
pinctrl-names = "default", "sleep";
pinctrl-0 = <&user_leds_s0>;
pinctrl-1 = <&user_leds_s1>;
user_leds_s0: user_leds_s0 {
pinctrl-single,pins = <
0x54 0x7/* gpmc_a5.gpio1_21, OUTPUT | MODE7 */
0x58 0x17 /* gpmc_a6.gpio1_22, OUT PULLUP | MODE7
*/
0x5c 0x7/* gpmc_a7.gpio1_23, OUTPUT | MODE7 */
0x60 0x17 /* gpmc_a8.gpio1_24, OUT PULLUP | MODE7
*/
>;
};
user_leds_s1: user_leds_s1 {
pinctrl-single,pins = <
0x54 0x2e /* gpmc_a5.gpio1_21, INPUT | MODE7 */
0x58 0x2e /* gpmc_a6.gpio1_22, INPUT | MODE7 */
0x5c 0x2e /* gpmc_a7.gpio1_23, INPUT | MODE7 */
0x60 0x2e /* gpmc_a8.gpio1_24, INPUT | MODE7 */
>;
};
};
I think "pinctrl-1" state will be used in driver
suspend/resume calls.
I have the pinmux/conf settings for default state but fully
optimized pinmux/conf values in idle & suspend states are not
available yet. Even though if I add here, I am unable to test
these pins in suspend state because suspend/resume of AM35xx
is not added yet
I have two options now
- add only default states for now, I can add reset of
the state details once suspend/resume is supported.
- add same values in all the states, modify those once
suspend/resume is added for AM335x.
Thanks
AnilKumar
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: [PATCH 1/2] Input: matrix-keypad - remove const from pointer to array of gpios
On Sat, Oct 20, 2012 at 04:30:59, Dmitry Torokhov wrote: > On Fri, Oct 19, 2012 at 12:36:12PM +0530, AnilKumar Ch wrote: > > Remove const from pointer to array of gpios in matrix_keypad_platform_data > > struct. This is required if we update row_gpios and col_gpios based on > > device tree data. > > Then don't. Set them up via non-const aliases instead. > Changed. Thanks AnilKumar ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: [PATCH 2/2] Input: matrix-keypad - Add device tree support
Hi Rob,
Thanks for the comments.
On Fri, Oct 19, 2012 at 18:27:21, Rob Herring wrote:
> On 10/19/2012 02:06 AM, AnilKumar Ch wrote:
> > Add device tree support to matrix keypad driver and usage details
> > are added to device tree documentation. Driver was tested on AM335x
> > EVM.
> >
> > Signed-off-by: AnilKumar Ch
> > ---
> > .../devicetree/bindings/input/matrix-keypad.txt| 52 ++
> > drivers/input/keyboard/matrix_keypad.c | 104
> > +++-
> > 2 files changed, 155 insertions(+), 1 deletion(-)
> > create mode 100644
> > Documentation/devicetree/bindings/input/matrix-keypad.txt
> >
> > diff --git a/Documentation/devicetree/bindings/input/matrix-keypad.txt
> > b/Documentation/devicetree/bindings/input/matrix-keypad.txt
> > new file mode 100644
> > index 000..50aaa6e
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/input/matrix-keypad.txt
> > @@ -0,0 +1,52 @@
> > +* GPIO driven matrix keypad device tree bindings
> > +
> > +GPIO driven matrix keypad is used to interface a SoC with a matrix keypad.
> > +The matrix keypad supports multiple row and column lines, a key can be
> > +placed at each intersection of a unique row and a unique column. The matrix
> > +keypad can sense a key-press and key-release by means of GPIO lines and
> > +report the event using GPIO interrupts to the cpu.
> > +
> > +Required Properties:
> > +- compatible: Should be "matix-keypad"
>
> How about gpio-matrix-keypad?
More meaningful, changed
>
> > +- keypad,num-row-gpios:Number of row lines connected to the keypad
> > + controller.
> > +- keypad,num-col-gpios:Number of column lines connected to the keypad
> > + controller.
>
> Isn't the number of gpios just the count of gpios listed below? So you
> don't need these props.
Removed
>
> > +- row-gpios: List of gpios used as row lines. The gpio
> > specifier
> > + for this property depends on the gpio controller to
> > + which these row lines are connected.
> > +- col-gpios: List of gpios used as column lines. The gpio
> > specifier
> > + for this property depends on the gpio controller to
> > + which these column lines are connected.
> > +
> > +Optional Properties:
> > +- linux,no-autorepeat: do no enable autorepeat feature.
> > +- linux,wakeup:use any event on keypad as wakeup event.
> > +- debounce-delay-ms: debounce interval in milliseconds
> > +- col-scan-delay-us: delay, measured in microseconds, that is needed
> > + before we can scan keypad after activating column gpio
> > +- clustered-irq: have clustered irq number
> > +- clustered-irq-flags: have clustered irq flags
>
> It's not clear what clustered means here. If I have to go read Linux
> code to understand, you are doing it wrong. Describe the h/w, not Linux
> data structs.
I have added based on pdata of the driver, I am not using these
parameters because AM335x-EVM have different interrupts for all
gpio pins.
clustered-irq: combined irq source for the whole matrix keypad,
like all gpio keys are connected to a gpio expander
>
> > +
> > +Example:
> > + matrix-keypad {
> > + compatible = "matrix-keypad";
> > + keypad,num-row-gpios = <3>;
> > + keypad,num-col-gpios = <2>;
> > + debounce-delay-ms = <5>;
> > + col-scan-delay-us = <2>;
> > +
> > + row-gpios = <&gpio2 25 0
> > +&gpio2 26 0
> > +&gpio2 27 0>;
> > +
> > + col-gpios = <&gpio2 21 0
> > +&gpio2 22 0>;
> > +
> > + linux,keymap = <0x008B
> > + 0x019E
> > + 0x0269
> > + 0x0001006A
> > + 0x0101001C
> > + 0x0201006C>;
> > + };
> > diff --git a/drivers/input/keyboard/matrix_keypad.c
> > b/drivers/input/keyboard/matrix_keypad.c
> > index 18b7237..39e480d 100644
> > --- a/drivers/input/keyboard/matrix_keypad.c
> > +++ b/drivers/input/keyboard/matrix_keypad.c
> > @@ -23,6 +23,9 @@
> > #include
> > #include
> > #include
> > +#include
> > +#include
> > +#include
> >
> > struct matrix_keypad {
> > const struct matrix_keypad_platform_data *pdata;
> > @@ -394,6 +397,91 @@ static void matrix_keypad_free_gpio(struct
> > matrix_keypad *keypad)
> > gpio_free(pdata->col_gpios[i]);
> > }
> >
> > +#ifdef CONFIG_OF
> > +static
> > +struct matrix_keypad_platform_data *matrix_keypad_parse_dt(struct device
> > *dev)
> > +{
> > + struct matrix_keypad_platform_data *pdata;
> > + struct matrix_keymap_data *keymap_data;
> > + struct device_node *np = dev->of_node;
> > + struct property *prop;
> > + int key_count = 0, length, row, col;
> > + uint32_t *keymap;
> > +
> > + pdat
RE: [RFC PATCH v3 13/16] ARM: dts: add AM33XX MMC support
On Thu, Oct 18, 2012 at 18:56:52, Porter, Matt wrote:
> Adds AM33XX MMC support for am335x-bone and am335x-evm.
>
> Signed-off-by: Matt Porter
> ---
> arch/arm/boot/dts/am335x-bone.dts |6 ++
> arch/arm/boot/dts/am335x-evm.dts |6 ++
> arch/arm/boot/dts/am33xx.dtsi | 27 +++
> 3 files changed, 39 insertions(+)
>
> diff --git a/arch/arm/boot/dts/am335x-bone.dts
> b/arch/arm/boot/dts/am335x-bone.dts
> index c634f87..5510979 100644
> --- a/arch/arm/boot/dts/am335x-bone.dts
> +++ b/arch/arm/boot/dts/am335x-bone.dts
> @@ -70,6 +70,8 @@
> };
>
> ldo3_reg: regulator@5 {
> + regulator-min-microvolt = <180>;
> + regulator-max-microvolt = <330>;
I think these min & max limits are regulator limits. Are these fields
required? Add details of these additions. AFAIK fine-tuned (board
specific) min/max limits should be add here(like mpu and core
regulator nodes)
> regulator-always-on;
> };
>
> @@ -78,3 +80,7 @@
> };
> };
> };
> +
> +&mmc1 {
> + vmmc-supply = <&ldo3_reg>;
> +};
> diff --git a/arch/arm/boot/dts/am335x-evm.dts
> b/arch/arm/boot/dts/am335x-evm.dts
> index 185d632..d63fce8 100644
> --- a/arch/arm/boot/dts/am335x-evm.dts
> +++ b/arch/arm/boot/dts/am335x-evm.dts
> @@ -114,7 +114,13 @@
> };
>
> vmmc_reg: regulator@12 {
> + regulator-min-microvolt = <180>;
> + regulator-max-microvolt = <330>;
=same=
> regulator-always-on;
> };
> };
> };
> +
> +&mmc1 {
> + vmmc-supply = <&vmmc_reg>;
> +};
> diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
> index ab9c78f..26a6af7 100644
> --- a/arch/arm/boot/dts/am33xx.dtsi
> +++ b/arch/arm/boot/dts/am33xx.dtsi
> @@ -234,6 +234,33 @@
> status = "disabled";
> };
>
> + mmc1: mmc@4806 {
> + compatible = "ti,omap3-hsmmc";
> + ti,hwmods = "mmc1";
> + ti,dual-volt;
> + ti,needs-special-reset;
> + dmas = <&edma 24
> + &edma 25>;
> + dma-names = "tx", "rx";
Add status = "disabled" here and "okay" in corresponding
.dts file
> + };
> +
> + mmc2: mmc@481d8000 {
> + compatible = "ti,omap3-hsmmc";
> + ti,hwmods = "mmc2";
> + ti,needs-special-reset;
> + dmas = <&edma 2
> + &edma 3>;
> + dma-names = "tx", "rx";
> + status = "disabled";
> + };
> +
> + mmc3: mmc@4781 {
> + compatible = "ti,omap3-hsmmc";
> + ti,hwmods = "mmc3";
> + ti,needs-special-reset;
What about DMA resources for mmc3?
AnilKumar
> + status = "disabled";
> + };
> +
> wdt2: wdt@44e35000 {
> compatible = "ti,omap3-wdt";
> ti,hwmods = "wd_timer2";
> --
> 1.7.9.5
>
> ___
> devicetree-discuss mailing list
> devicetree-discuss@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/devicetree-discuss
>
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: [PATCH] ARM: dts: AM33XX: Add tsl2550 ambient light sensor DT data
+Grant, DTML
On Thu, Oct 18, 2012 at 13:38:30, Cousson, Benoit wrote:
> Hi Anil,
>
> On 10/18/2012 07:46 AM, AnilKumar, Chimata wrote:
> > On Fri, Sep 21, 2012 at 21:19:11, AnilKumar, Chimata wrote:
> >> Add tsl2550 ambient light sensor DT data to am335x-evm.dts. In AM335x
> >> EVM tsl2550 ambient light sensor is connected to I2C2 bus. So this patch
> >> adds child node inside i2c2 node with i2c slave address.
> >>
> >> TAOS tsl2550 sensor with two-wire SMBus serial interface. This patch
> >> also reduces I2C2 clock frequency to 100KHz from 400KHz because the
> >> maximum clock frequency of SMBus is 100KHz.
> >>
> >> Signed-off-by: AnilKumar Ch
> >> ---
> >> arch/arm/boot/dts/am335x-evm.dts |7 ++-
> >> 1 file changed, 6 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/arch/arm/boot/dts/am335x-evm.dts
> >> b/arch/arm/boot/dts/am335x-evm.dts
> >> index 3b1f313..d99aa0f 100644
> >> --- a/arch/arm/boot/dts/am335x-evm.dts
> >> +++ b/arch/arm/boot/dts/am335x-evm.dts
> >> @@ -49,7 +49,7 @@
> >>
> >>i2c2: i2c@4802a000 {
> >>status = "okay";
> >> - clock-frequency = <40>;
> >> + clock-frequency = <10>;
> >>
> >>lis331dlh: lis331dlh@18 {
> >>compatible = "st,lis331dlh", "st,lis3lv02d";
> >> @@ -79,6 +79,11 @@
> >>st,max-limit-z = <750>;
> >>};
> >>
> >> + tsl2550: tsl2550@39 {
> >> + compatible = "taos,tsl2550";
> >> + reg = <0x39>;
> >> + };
> >> +
> >>tmp275: tmp275@48 {
> >>compatible = "ti,tmp275";
> >>reg = <0x48>;
> >
> > Hi Tony/Benoit,
> >
> > If there are no comments in this patch could you please take this in?
>
> Have you updated the binding documentation to list the device?
>
> It should be in:
> Documentation/devicetree/bindings/i2c/trivial-devices.txt
>
No I have not updated, I will send a separate patch by adding all the supported
devices, missed from earlier (this device, tmp275 and lis331dlh)
Thanks
AnilKumar
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: [PATCH v2] leds: leds-gpio: adopt pinctrl support
On Tue, Oct 02, 2012 at 01:29:37, Linus Walleij wrote: > On Mon, Oct 1, 2012 at 5:44 PM, Tony Lindgren wrote: > > >> OK that is typical pinctrl driver implementation work. > >> I hope Tony can advice on this? > > > > I think we're best off to just stick to alternative named modes > > passed from device tree. For example, for GPIO wake-ups you can > > have named modes such as "default", "enabled" and "idle" where > > "idle" muxes things for GPIO wake-ups for the duration of idle. > > In this case we need to add three different values according to three modes (default, enabled, idle) and for each node. > > It seems that should also work for leds-gpio. And you can > > define more named modes as needed. If we want to implement pinctrl_gpio functionality we have to separate "function-mask" bits to 1. pinmux-mask 2. pinconf-mask, to make it generic we need following bit masks a. receiver enable/disable bit b. slew rate fast/slow bit c. pull-up/down bit I have gone through nvidia pinctrl dt data (tegra20-seaboard.dts, node drive_sdio1) which has different pinconfig values, those are mapping to pinconf values. With the above bit masks and function-mask we can identify pull-up/down, slow/high speed slew rate and direction in/out. (or) Named modes:- Are you saying named modes like this? default-input-up default-input-down default-output-up default-output-down This 1, 2 and 2.a or named modes are required to implement pinctrl_gpio_direction_input/output and pinctrl_request/free_gpio. > > > This is what we're doing for ux500 and should be a good model. I have looked into this, but not seen any named modes. Thanks AnilKumar ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: [PATCH v2] leds: leds-gpio: adopt pinctrl support
+Don Aisheng On Tue, Sep 11, 2012 at 01:10:12, Linus Walleij wrote: > On Sat, Sep 1, 2012 at 10:16 AM, AnilKumar Ch wrote: > > > Adopt pinctrl support to leds-gpio driver based on leds-gpio > > device pointer, pinctrl driver configure SoC pins to GPIO > > mode according to definitions provided in .dts file. > > > > Signed-off-by: AnilKumar Ch > > So looking back at this after Stephen posed a real good > question, when you say "configure SoC pins to GPIO > mode", does that mean anything else than to mux them into > GPIO mode? > pinctrl-single.c driver sets mux mode as well as pin configuration like pull-up/pull-down, input/output and slew rate. > In that case, have you considered augmenting > pinctrl-single.c to implement .gpio_request_enable() > .gpio_disable_free() and maybe also .gpio_set_direction() > in its struct pinmux_ops pinmux backend? > > If not, why? Recently, I have gone through the details on implementing gpio_request_enable in pinctrl-single driver. To add this functionality we have to add gpio_range's to pinctrl driver. AM335x EVM supports total 128 GPIO pins (4 banks) and these are randomly distributed across AM33XX SoC pins. These are the concerns/questions I have:- 1. I haven't added yet but it will come to more than 30-40 pinctrl_gpio_range entries 2. If the silicon package is changed then these will change. 3. If the GPIO driver is common for multiple SoCs then these entries will be more & more over SoC specific and driver has to change every time the gpio_range changes. I have gone through the "Don Aisheng" patch series, which adds "pinctrl_dt_add_gpio_ranges" support but not accepted yet. With this patch series we can overcome the driver changes. 4. The current pinctrl driver has support for these APIs pinctrl_request_gpio(), pinctrl_free_gpio(), pinctrl_gpio_direction_input/output() no API for slew rate control, pulled down/up control how can we handle this? 5. pinctrl-single driver has to modify to provide separate handles for pinmux and pinconfig. And we need separate pin configuration bit masks and values/flags to take care of pull-up/down, slew rate, receiver in/out and mux mode control. 6. This is for my understanding, on which node do we have to add pinctrl data i.e., pin mux-mode data. In leds-gpio node (mostly not) and if it is in gpio node then how can we pass pinmux data with the existing API pinctrl_request_gpio(gpio);? Here we are passing only gpio number. Few more driver patches are pending along with this (leds-gpio DT data additions according to this patch, similarly other drivers like matrix keypad and volume keys) Thanks AnilKumar ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: [PATCH v3 4/4] ARM: dts: AM33XX: Add lis331dlh device tree data to am335x-evm
Hi Andrew, On Tue, Sep 18, 2012 at 04:05:59, Andrew Morton wrote: > On Mon, 17 Sep 2012 12:53:22 +0530 > AnilKumar Ch wrote: > > > Add lis331dlh device tree data to am335x-evm.dts. In AM335x EVM > > lis331dlh accelerometer is connected to I2C2 bus. So this patch > > change the status of I2C2 node to "okay" to use I2C2 bus. Also > > added all the required platform data to am335x-evm. > > > > Signed-off-by: AnilKumar Ch > > --- > > arch/arm/boot/dts/am335x-evm.dts | 39 > > ++ > > Your arch/arm/boot/dts/am335x-evm.dts differs significantly from the > one in linux-next (it should not do so), so I didn't do anything with > this patch. > Thanks for pushing remaining patches. ACK from the reviewers(Arnd) will help to include in linux-omap branch. Thanks AnilKumar ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: [PATCH 2/2] ARM: dts: AM33XX: Add lis331dlh device tree data to am335x-evm
+Daniel
On Fri, Sep 14, 2012 at 15:58:37, Arnd Bergmann wrote:
> On Friday 14 September 2012, AnilKumar, Chimata wrote:
> >> On Fri, Sep 14, 2012 at 13:56:06, Arnd Bergmann wrote:
> > > On Thursday 13 September 2012, AnilKumar Ch wrote:
>
> > >
> > > Why do you put the "reg" property here
> >
> > Here I specified reg property because lis331dlh I2C slave address is 0x18.
> >
> > >
> > > > dcan1: d_can@481d {
> > > > status = "okay";
> > > > pinctrl-names = "default";
> > > > @@ -61,6 +70,39 @@
> > > > regulator-max-microvolt = <500>;
> > > > regulator-boot-on;
> > > > };
> > > > +
> > > > + lis3_reg: fixedregulator@1 {
> > > > + compatible = "regulator-fixed";
> > > > + regulator-name = "lis3_reg";
> > > > + regulator-boot-on;
> > > > + };
> > > > +};
> > > > +&lis331dlh {
> > > > + compatible = "st,lis3lv02d-i2c";
> > >
> > > and all the rest here? At least I would expect the "compatible" property
> > > to be in the same place above.
> >
> > This data is appended to above one, to make it readable I moved remaining
> > properties to here.
>
> I don't follow how this is making things more readable.
Basic lis3 parameter is in i2c2 node to tell the hierarchy of
nodes and remaining parameters at the bottom. With this way
we can understand easily what is start & end braces so that
node hierarchy is cleaner. (This is my view)
>
> Maybe a more logical way to do this would be use the existing i2c2 label
> and write all the additions as
>
> i2c2: {
> status = "okay";
> clock-frequency = <40>;
>
> lis331dlh@18 {
> compatible = "st,lis3lv02d";
> reg = <0x18>;
>
> vdd-supply = <&lis3_reg>;
> vdd-io-supply = <&lis3_reg>;
> ...
> };
>
If this is better/preferable way then I will change.
> > > Also, I think you should remove the "-i2c" postfix from the name, that
> > > is already implied by the parent bus.
> >
> > I will remove, but in case of spi the compatible name is lis3lv02d_spi.
> > By mistake I have uses "-i2c" instead of "_i2c".
Then, I will change to 'st,lis3lv02d' in lis3lv02d_i2c driver
and same will added to .dts file.
Small question here, in my v2 version I have specified both
the compatible names lis3lv02d and lis331dlh is it fine or
only one is sufficient?
+static struct of_device_id lis3lv02d_i2c_dt_ids[] = {
+ { .compatible = "st,lis3lv02d" },
+ { .compatible = "st,lis331dlh" },
+ {}
+};
>
> The normal convention is to use '-', not '_', so that part was ok.
>
> I think naming the other one lis3lv02d_spi was a mistake, it should
> be named 'st,lis3lv02d' independent of the bus IMHO.
Yes, initially I expected the same. I think driver name is used
as a compatible string.
>
> > Document is already present,
> > http://git.kernel.org/?p=linux/kernel/git/next/linux-next.git;a=commit;h=2f2ff3cc8d930493f9a598b9192706c09403e12e
> >
> > Some minor changes in docs, in my next version I will update document
> > as well. I will send V3 if there are no comments on v2.
>
Can you just quickly review v2 series? So that I will send
V3.
Thanks
AnilKumar
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: [PATCH 2/2] ARM: dts: AM33XX: Add lis331dlh device tree data to am335x-evm
Hi Arnd,
Thanks for the review,
On Fri, Sep 14, 2012 at 13:56:06, Arnd Bergmann wrote:
> On Thursday 13 September 2012, AnilKumar Ch wrote:
> > Add lis331dlh device tree data to am335x-evm.dts. In AM335x EVM
> > lis331dlh accelerometer is connected to I2C2 bus. So this patch
> > change the status to "okay" to use I2C2 bus. Also added all the
> > required platform data to am335x-evm.
> >
> > Signed-off-by: AnilKumar Ch
> > ---
> > arch/arm/boot/dts/am335x-evm.dts | 42
> > ++
> > 1 file changed, 42 insertions(+)
> >
> > diff --git a/arch/arm/boot/dts/am335x-evm.dts
> > b/arch/arm/boot/dts/am335x-evm.dts
> > index 9fb59c5..9e5a878 100644
> > --- a/arch/arm/boot/dts/am335x-evm.dts
> > +++ b/arch/arm/boot/dts/am335x-evm.dts
> > @@ -47,6 +47,15 @@
> > };
> > };
> >
> > + i2c2: i2c@4802a000 {
> > + status = "okay";
> > + clock-frequency = <40>;
> > +
> > + lis331dlh: lis331dlh@18 {
> > + reg = <0x18>;
> > + };
> > + };
>
> Why do you put the "reg" property here
Here I specified reg property because lis331dlh I2C slave address is 0x18.
>
> > dcan1: d_can@481d {
> > status = "okay";
> > pinctrl-names = "default";
> > @@ -61,6 +70,39 @@
> > regulator-max-microvolt = <500>;
> > regulator-boot-on;
> > };
> > +
> > + lis3_reg: fixedregulator@1 {
> > + compatible = "regulator-fixed";
> > + regulator-name = "lis3_reg";
> > + regulator-boot-on;
> > + };
> > +};
> > +&lis331dlh {
> > + compatible = "st,lis3lv02d-i2c";
>
> and all the rest here? At least I would expect the "compatible" property
> to be in the same place above.
This data is appended to above one, to make it readable I moved remaining
properties to here.
>
> Also, I think you should remove the "-i2c" postfix from the name, that
> is already implied by the parent bus.
I will remove, but in case of spi the compatible name is lis3lv02d_spi.
By mistake I have uses "-i2c" instead of "_i2c".
>
> > + Vdd-supply = <&lis3_reg>;
> > + Vdd_IO-supply = <&lis3_reg>;
> > +
> > + st,click-single-x;
> > + st,click-single-y;
> > + st,click-single-z;
> > + st,click-thresh-x = <10>;
> > + st,click-thresh-y = <10>;
> > + st,click-thresh-z = <10>;
> > + st,irq1-click;
> > + st,irq2-click;
> > + st,wakeup-x-lo;
> > + st,wakeup-x-hi;
> > + st,wakeup-y-lo;
> > + st,wakeup-y-hi;
> > + st,wakeup-z-lo;
> > + st,wakeup-z-hi;
> > + st,min-limit-x = <120>;
> > + st,min-limit-y = <120>;
> > + st,min-limit-z = <140>;
> > + st,max-limit-x = <550>;
> > + st,max-limit-y = <550>;
> > + st,max-limit-z = <750>;
>
> Is there a binding document that describes all these?
Document is already present,
http://git.kernel.org/?p=linux/kernel/git/next/linux-next.git;a=commit;h=2f2ff3cc8d930493f9a598b9192706c09403e12e
Some minor changes in docs, in my next version I will update document
as well. I will send V3 if there are no comments on v2.
Thanks
AnilKumar
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: [PATCH 2/2] arm/dts: AM33XX: Add device tree OPP table
On Wed, Sep 12, 2012 at 22:51:55, Cousson, Benoit wrote: > + Paul > > Hi Anil, > > On 08/31/2012 11:37 AM, AnilKumar Ch wrote: > > Add DT OPP table for AM33XX family of devices. This data is > > decoded by OF with of_init_opp_table() helper function. > > > > Also adds cpu0 supply name to the corresponding dts files. > > cpu0-supply name is used by cpufreq-cpu0 driver to get the > > regulator pointer for voltage modifications. > > > > Signed-off-by: AnilKumar Ch > > I've just applied your patch in my for_3.7/dts_part2 branch. Thanks Benoit, > > I changed the subject to use "ARM: dts: " prefix seems it seems to be > the convention nowadays. I will take care from next time onwards. > > I can apply as the well the clock patch if Paul acks it, but it can go > through Paul as well since there is no strong dependency between them AFAIK. Paul, Can you ACK "clock data entry" patch? Thanks AnilKumar ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: [PATCH v2] leds: leds-gpio: adopt pinctrl support
On Fri, Sep 07, 2012 at 16:32:51, Domenico Andreoli wrote:
> On Fri, Sep 07, 2012 at 09:10:50AM +0000, AnilKumar, Chimata wrote:
> > Hi Domenico,
>
> Hi,
>
> > On Fri, Sep 07, 2012 at 14:18:39, Domenico Andreoli wrote:
> > > On Sat, Sep 1, 2012 at 10:16 AM, AnilKumar Ch wrote:
> > > > Adopt pinctrl support to leds-gpio driver based on leds-gpio
> > > > device pointer, pinctrl driver configure SoC pins to GPIO
> > > > mode according to definitions provided in .dts file.
> > >
> > > Shouldn't be the interaction with the pinctrl layer left to gpiolib?
> > >
> >
> > No, these gpio's are configured specifically for user leds.
>
> So there are some special pad configs to make the leds work which are not
> only muxing and direction setting? Because I expect these to be managed
> privately between gpiolib and pinctrl but now I'm not sure any more,
> I'll look the code.
How can gpio driver knows that leds-gpio driver require
these 4 pins?
>
> > So, leds-gpio driver should have this call, because these gpio
> > pins are used by leds-gpio driver.
> >
> > + am33xx_pinmux: pinmux@44e10800 {
> > + userled_pins: pinmux_userled_pins {
> > + pinctrl-single,pins = <
> > + 0x54 0x7
> > + 0x58 0x17
> > + 0x5c 0x7
> > + 0x60 0x17
> > + >;
> > + };
> > + };
> > +
> >
> > [...]
> >
> > + leds {
> > + compatible = "gpio-leds";
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&userled_pins>;
> >
>
> I'm surprised to not see any gpio controller (ala irq) involved.
GPIO controller data will be in GPIO node, should not be here.
>
> > Lets take gpio-keypad driver, in that case we have to configure
> > pins as INPUT mode (generic gpio driver might not know what
> > the end usecase is) and this leds case we configure as OUTPUT
> > mode.
>
> gpio direction is modeled by gpiolib so, if no other out-of-gpiolib
> capabilities are required for that led gpio, there is no need to directly
> use pinctrl.
>
Here leds-gpio driver requirement is to set mux configuration of
those 4 pins to GPIO mode (mode 7) as well direction OUTPUT/INPUT.
Set mux mode to 7 (GPIO usage) should be from led driver, because
this driver have the requirement.
Thanks
AnilKumar
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: [PATCH v2] leds: leds-gpio: adopt pinctrl support
Hi Domenico,
On Fri, Sep 07, 2012 at 14:18:39, Domenico Andreoli wrote:
> On Sat, Sep 1, 2012 at 10:16 AM, AnilKumar Ch wrote:
> > Adopt pinctrl support to leds-gpio driver based on leds-gpio
> > device pointer, pinctrl driver configure SoC pins to GPIO
> > mode according to definitions provided in .dts file.
>
> Shouldn't be the interaction with the pinctrl layer left to gpiolib?
>
No, these gpio's are configured specifically for user leds.
So, leds-gpio driver should have this call, because these gpio
pins are used by leds-gpio driver.
+ am33xx_pinmux: pinmux@44e10800 {
+ userled_pins: pinmux_userled_pins {
+ pinctrl-single,pins = <
+ 0x54 0x7
+ 0x58 0x17
+ 0x5c 0x7
+ 0x60 0x17
+ >;
+ };
+ };
+
[...]
+ leds {
+ compatible = "gpio-leds";
+ pinctrl-names = "default";
+ pinctrl-0 = <&userled_pins>;
This devm_pinctrl_get_select_default() call in leds-gpio driver
will internally take userled_pins node and configure those pins
according to the above definitions.
Lets take gpio-keypad driver, in that case we have to configure
pins as INPUT mode (generic gpio driver might not know what
the end usecase is) and this leds case we configure as OUTPUT
mode.
Thanks
AnilKumar
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: [PATCH v2] leds: leds-gpio: adopt pinctrl support
On Fri, Sep 07, 2012 at 05:39:35, Marek Vasut wrote: > Dear Tony Lindgren, > > > * Marek Vasut [120905 19:05]: > > > Hi Tony, > > > > > > > * Marek Vasut [120904 20:13]: > > > > > Dear Bryan Wu, > > > > > > > > > > > On Sat, Sep 1, 2012 at 4:16 PM, AnilKumar Ch > > > > > > wrote: > > > > > > > Adopt pinctrl support to leds-gpio driver based on leds-gpio > > > > > > > device pointer, pinctrl driver configure SoC pins to GPIO > > > > > > > mode according to definitions provided in .dts file. > > > > > > > > > > > > Thanks for this, actually Marek Vasut submitted a similar patch > > > > > > before. I'm pretty fine with this patch. > > > > > > > > > > Thanks for submitting this actually ... I didn't have time to > > > > > properly investigate this. > > > > > > > > > > > But without proper DT setting, it will also give us warning I > > > > > > think. or we can provide some dummy functions as a temp solution > > > > > > as Shawn pointed out before. > > > > > > > > > > But this driver is also used on hardware that's not yet coverted to > > > > > DT, so I'd say dev_warn() if CONFIG_OF is enabled and otherwise > > > > > simply go on ? Actually, can we not skip whole this pinctrl thing if > > > > > CONFIG_OF is disabled? Actually (2), what's the relationship between > > > > > OF and pinctrl? > > > > > > > > The warning should be pinctrl related as the pinctrl drivers may not be > > > > device tree based drivers. > > > > > > Exactly my concern. Also the warning shouldnt be present on systems where > > > pinctrl is disabled. > > > > But pinctrl_get_select() returns 0 in include/linux/pinctrl/consumer.h if > > CONFIG_PINCTRL is not selected, so no warning is produced AFAIK ;) > > Oh all right then. > Bryan, If this patch looks fine, can you queue this for 3.7? Thanks AnilKumar ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: [PATCH v2 1/8] ARM/dts: OMAP2: Add McBSP entries for OMAP2420 and OMAP2430 SoC
On Wed, Sep 05, 2012 at 20:11:11, Hiremath, Vaibhav wrote: > On Wed, Sep 05, 2012 at 19:01:55, Cousson, Benoit wrote: > > Hi Vaibhav, > > > > On 09/05/2012 11:15 AM, Hiremath, Vaibhav wrote: > > > On Wed, Sep 05, 2012 at 13:53:58, Ujfalusi, Peter wrote: > > >> Hi, > > >> > > >> On 09/05/2012 09:11 AM, Hiremath, Vaibhav wrote: > > Not yet, but we discussed that with Peter and since he does have these > > patches for DT, he'll be able to test your series with the McBSP > > changes. > > > > >>> > > >>> Great. > > >> > > >> With his series and your patch for omap-hwmod audio was probing and > > >> working on > > >> OMAP3/4/5 without issues. > > >> > > > Peter, > > > Care to provide your Tested-By?? > > > > > > Benoit, > > > Can we merge this patch now? > > > > Yes, I'll include it in the pull request along with DTS patches. > > > > I've just tested it as well on OMAP4 by hacking the DTS for GPIO. > > I'll try to update at least all the OMAP4 IPs as well with the proper > > DTS resources for 3.7. > > > > Thanks Benoit, > > There are other patches which are pending, > > arm/dts: AM33XX: Convert all hex numbers to lower-case > https://patchwork.kernel.org/patch/1377351/ > arm/dts: AM33XX: Specify reg and interrupt property for all nodes > https://patchwork.kernel.org/patch/1377361/ > Few more DT patches which are pending arm/dts: AM33XX: Add basic pinctrl device tree data http://www.spinics.net/lists/linux-omap/msg76684.html arm/dts: AM33XX: Configure pinmuxs for user leds control on Bone http://www.spinics.net/lists/linux-omap/msg76682.html arm/dts: AM33XX: Add D_CAN device tree data http://www.spinics.net/lists/linux-omap/msg76683.html Thanks AnilKumar ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: [PATCH v7 0/3] arm/dts: Add device tree data for AM33XX devices
On Thu, Sep 06, 2012 at 15:08:21, AnilKumar, Chimata wrote: > Add pinctrl and d_can device tree data to AM33XX family of devices. > First two patches add support for pinctrl DT data and third one > adds dcan DT data. > > Reason behind combining these patches is to apply cleanly on > linux-omap tree, because these are sequential patches. > > These patches were tested on AM335x-Bone, AM335x-EVM and based > on linux-omap:master with this patch > http://www.mail-archive.com/linux-omap@vger.kernel.org/msg74393.html > If there are no changes in this patch series ACK from reviewers (esp. Tony, koen and Marc) will help. Thanks AnilKumar ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: [PATCH v6 2/3] arm/dts: AM33XX: Configure pinmuxs for user leds control on Bone
Tony,
On Fri, Sep 07, 2012 at 02:28:12, Tony Lindgren wrote:
> * AnilKumar Ch [120905 04:42]:
> > Adds GPIO pinctrl nodes to am3358_pinmux master node to control
> > user leds (USR0, USR1, USR2 and USR3) present on BeagleBone.
> >
> > [k...@dominion.thruhere.net: led0, led1 suggested by koen]
> > Signed-off-by: AnilKumar Ch
> > ---
> > arch/arm/boot/dts/am335x-bone.dts | 43
> > +
> > 1 file changed, 43 insertions(+)
> >
> > diff --git a/arch/arm/boot/dts/am335x-bone.dts
> > b/arch/arm/boot/dts/am335x-bone.dts
> > index c634f87..822efe6 100644
> > --- a/arch/arm/boot/dts/am335x-bone.dts
> > +++ b/arch/arm/boot/dts/am335x-bone.dts
> > @@ -18,11 +18,54 @@
> > reg = <0x8000 0x1000>; /* 256 MB */
> > };
> >
> > + am3358_pinmux: pinmux@44e10800 {
> > + userled_pins: pinmux_userled_pins {
> > + pinctrl-single,pins = <
> > + 0x54 0x7/* gpmc_a5.gpio1_21, OUTPUT |
> > MODE7 */
> > + 0x58 0x17 /* gpmc_a6.gpio1_22,
> > OUTPUT_PULLUP | MODE7 */
> > + 0x5c 0x7/* gpmc_a7.gpio1_23, OUTPUT |
> > MODE7 */
> > + 0x60 0x17 /* gpmc_a8.gpio1_24,
> > OUTPUT_PULLUP | MODE7 */
> > + >;
> > + };
> > + };
> > +
>
> Just checking.. am3358_pinmux should be am33xx_pinmux in this patch
> too as discussed.
>
Yes, this is also changed in v7.
Thanks
AnilKumar
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: [PATCH RESEND v5 3/3] arm/dts: AM33XX: Add D_CAN device tree data
On Wed, Sep 05, 2012 at 19:16:58, Marc Kleine-Budde wrote:
> On 09/05/2012 01:41 PM, AnilKumar Ch wrote:
> > Add Bosch D_CAN controller device tree data to AM33XX dtsi
> > file by adding d_can device nodes with all the necessary
> > parameters.
> >
> > Signed-off-by: AnilKumar Ch
> > ---
> > arch/arm/boot/dts/am33xx.dtsi | 18 ++
> > 1 file changed, 18 insertions(+)
> >
> > diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
> > index bf5f713..ab744d6 100644
> > --- a/arch/arm/boot/dts/am33xx.dtsi
> > +++ b/arch/arm/boot/dts/am33xx.dtsi
> > @@ -177,5 +177,23 @@
> > compatible = "ti,omap3-wdt";
> > ti,hwmods = "wd_timer2";
> > };
> > +
> > + dcan0: d_can@481cc000 {
> > + compatible = "bosch,d_can";
>
> On imx/mxs we also add a compatible for the current hardware, this may
> look like this:
>
> compatible = "ti,am33xx-can", "bosch,d_can";
>
> But I don't know if this is a generic policy or just an imx/mxs thing :)
>
Marc,
No driver compatible name is ip specific so "ti,am33xx-can" is not
required, I am not seeing any advantage if we add this name.
If there are any changes required for am33xx specific in driver
then we can add "ti,am33xx-can" name in driver otherwise no use.
Thanks
AnilKumar
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: [PATCH v5 1/5] arm/dts: AM33XX: Add basic pinctrl device tree data
On Thu, Sep 06, 2012 at 05:02:05, Tony Lindgren wrote:
> * AnilKumar Ch [120831 02:30]:
> > Adds basic pinctrl device tree data for AM33XX family of devices.
> > This patch is based on the pinctrl-single driver.
> >
> > Signed-off-by: AnilKumar Ch
> > ---
> > arch/arm/boot/dts/am33xx.dtsi |9 +
> > 1 file changed, 9 insertions(+)
> >
> > diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
> > index be43511..bf5f713 100644
> > --- a/arch/arm/boot/dts/am33xx.dtsi
> > +++ b/arch/arm/boot/dts/am33xx.dtsi
> > @@ -40,6 +40,15 @@
> > };
> > };
> >
> > + am3358_pinmux: pinmux@44e10800 {
> > + compatible = "pinctrl-single";
> > + reg = <0x44e10800 0x0238>;
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + pinctrl-single,register-width = <32>;
> > + pinctrl-single,function-mask = <0x7f>;
> > + };
> > +
>
> Is this controller the same for all am33xx? If so, please use
> am33xx_pinmux naming. Note that some padconf registers may not
> be listed for all the variants, but the registers may still be
> there for all the variants. So a generic entry is a better
> choice here as otherwise you'l need to include am33xx.dtsi into
> am3358.dtsi.
>
Controller is same for all am33xx devices, I will spin a patch
by renaming to am33xx_
Thanks
AnilKumar
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: [PATCH v3 3/3] cpufreq: Add a generic cpufreq-cpu0 driver
On Wed, Sep 05, 2012 at 06:42:21, Shawn Guo wrote: > On Wed, Sep 05, 2012 at 01:18:38AM +0200, Rafael J. Wysocki wrote: > > On Friday, August 10, 2012, Shawn Guo wrote: > > > It adds a generic cpufreq driver for CPU0 frequency management based on > > > clk, regulator, OPP and device tree support. It can support both > > > uniprocessor (UP) and those symmetric multiprocessor (SMP) systems which > > > share clock and voltage across all CPUs. > > > > > > Signed-off-by: Shawn Guo > > > > I seem to recall that this has been discussed with several people and > > undergone some changes as a result of comments. Would it be possible > > to get some ACKs from the people involved? > > > > Mark, Santosh, > > Reviewed-by, please? > > AnilKumar, > > Tested-by, please? I have tested on AM335x-EVM and AM335x-Bone devices. Tested-by: AnilKumar Ch > > > > > > --- /dev/null > > > +++ b/drivers/cpufreq/cpufreq-cpu0.c > > > @@ -0,0 +1,271 @@ > > > +/* > > > + * Copyright (C) 2012 Freescale Semiconductor, Inc. > > > + * > > > + * Reuse some codes from drivers/cpufreq/omap-cpufreq.c > > > > What do you mean by "reuse" here? > > > cpu0_set_target() function is almost a copy of omap_target() from > drivers/cpufreq/omap-cpufreq.c. I should probably just say so? > > > > + * > > > + * The code contained herein is licensed under the GNU General Public > > > + * License. You may obtain a copy of the GNU General Public License > > > + * Version 2 or later at the following locations: > > > > The kernel is just GPLv2 (not later) and it will stay this way. Please > > update this comment accordingly. > > > Okay. > > > > + * > > > + * http://www.opensource.org/licenses/gpl-license.html > > > + * http://www.gnu.org/copyleft/gpl.html > > > + */ > > -- > Regards, > Shawn > ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: [PATCH RESEND v4 2/3] arm/dts: AM33XX: Configure pinmuxs for user leds control on Bone
Hi Koen,
On Sat, Sep 01, 2012 at 18:21:10, AnilKumar, Chimata wrote:
> Hi Koen,
>
> On Sat, Sep 01, 2012 at 16:17:35, Koen Kooi wrote:
> >
> >
> > Op 1 sep. 2012 om 09:01 heeft "AnilKumar, Chimata" het
> > volgende geschreven:
> >
> > > Hi Koen,
> > >
> > > On Fri, Aug 31, 2012 at 21:23:18, Koen Kooi wrote:
> > >>
> > >> Op 30 aug. 2012, om 22:35 heeft Tony Lindgren het
> > >> volgende geschreven:
> > >>
> > >>> * AnilKumar Ch [120828 01:11]:
> > >>>> Adds GPIO pinctrl nodes to am3358_pinmux master node to control
> > >>>> user leds (USR0, USR1, USR2 and USR3) present on BeagleBone.
> > >>>>
> > >>>> Signed-off-by: AnilKumar Ch
> > >>>> ---
> > >>>> arch/arm/boot/dts/am335x-bone.dts | 14 ++
> > >>>> 1 file changed, 14 insertions(+)
> > >>>>
> > >>>> diff --git a/arch/arm/boot/dts/am335x-bone.dts
> > >>>> b/arch/arm/boot/dts/am335x-bone.dts
> > >>>> index a7906cb..58f5042 100644
> > >>>> --- a/arch/arm/boot/dts/am335x-bone.dts
> > >>>> +++ b/arch/arm/boot/dts/am335x-bone.dts
> > >>>> @@ -18,6 +18,20 @@
> > >>>>reg = <0x8000 0x1000>; /* 256 MB */
> > >>>>};
> > >>>>
> > >>>> +am3358_pinmux: pinmux@44E10800 {
> > >>>> +pinctrl-names = "default";
> > >>>> +pinctrl-0 = <&userled_pins>;
> > >>>> +
> > >>>> +userled_pins: pinmux_userled_pins {
> > >>>> +pinctrl-single,pins = <
> > >>>> +0x54 0x7/* gpmc_a5.gpio1_21, OUTPUT | MODE7 */
> > >>>> +0x58 0x17/* gpmc_a6.gpio1_22, OUTPUT_PULLUP |
> > >>>> MODE7 */
> > >>>> +0x5C 0x7/* gpmc_a7.gpio1_23, OUTPUT | MODE7 */
> > >>>> +0x60 0x17/* gpmc_a8.gpio1_24, OUTPUT_PULLUP |
> > >>>> MODE7 */
> > >>>> +>;
> > >>>> +};
> > >>>> +};
> > >>>> +
> > >>>
> > >>> Looks like this patch should also claim these pins by the led driver.
> > >>> Then the led driver should just do
> > >>> pinctrl_get_select_default(&pdev->dev)
> > >>> in it's probe function to set the pins.
> > >>
> > >> FWIW, I've been using this for a while now:
> > >>
> > >> + leds {
> > >> + compatible = "gpio-leds";
> > >> + heartbeat {
> > >> + label = "beaglebone::usr0";
> > >> + gpios = <&gpio2 21 0>;
> > >> + linux,default-trigger = "heartbeat";
> > >> + };
> > >> +
> > >> + mmc {
> > >> + label = "beaglebone:usr1";
> > >> + gpios = <&gpio2 22 0>;
> > >> + linux,default-trigger = "mmc0";
> > >> + };
> > >
> > > Thanks for the inputs, similar data but not exact is added to
> > > v5 series.
> > >
> > > +gpio-leds {
> > > +compatible = "gpio-leds";
> > > +pinctrl-names = "default";
> > > +pinctrl-0 = <&userled_pins>;
> > > +
> > > +led0 {
> > > +label = "status:green:user0";
> > > +gpios = <&gpio2 21 0>;
> > > +default-state = "off";
> > > +};
> > > +
> > > +led1 {
> > > +label = "status:green:user1";
> > > +gpios = <&gpio2 22 0>;
> > > +default-state = "off";
> > > +};
> > > +
> > > +led2 {
> > > +label = "status:green:user2";
> > > +gpios = <&gpio2 23 0>;
> > > +default-state = "off";
> > > +};
> >
RE: [PATCH 5/5] of: Modify c_can binding documentation
Hi Stephen, Thanks for the review, On Sun, Sep 02, 2012 at 07:32:38, Stephen Warren wrote: > On 09/01/2012 12:05 AM, AnilKumar, Chimata wrote: > > On Fri, Aug 31, 2012 at 14:59:21, AnilKumar, Chimata wrote: > >> Modify c_can binding documentation according to recent review comments > >> on device tree data addition patches. > > >> diff --git a/Documentation/devicetree/bindings/net/can/c_can.txt > >> b/Documentation/devicetree/bindings/net/can/c_can.txt > >> index a43f083..90a70be 100644 > >> --- a/Documentation/devicetree/bindings/net/can/c_can.txt > >> +++ b/Documentation/devicetree/bindings/net/can/c_can.txt > >> @@ -8,6 +8,8 @@ Required properties: > >> registers map > >> - interrupts : property with a value describing the interrupt > >> number > >> +- status : describes the node status either "disabled" or > >> +"okay" > > That's a standrd property that applies to any node, and doesn't describe > data required by the device itself (as do regs/interrupts) by simply > whether the node is activated; I'm not sure it's worth mentioning it in > a device-specific binding. > > A similar comment exists for the pre-existing description of > interrupt-parent below. > > >> Optional properties: > >> - interrupt-parent: The parent interrupt controller > Then, I will remove these properties from the doc. Thanks AnilKumar ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: [PATCH RESEND v4 2/3] arm/dts: AM33XX: Configure pinmuxs for user leds control on Bone
Hi Koen,
On Sat, Sep 01, 2012 at 16:17:35, Koen Kooi wrote:
>
>
> Op 1 sep. 2012 om 09:01 heeft "AnilKumar, Chimata" het
> volgende geschreven:
>
> > Hi Koen,
> >
> > On Fri, Aug 31, 2012 at 21:23:18, Koen Kooi wrote:
> >>
> >> Op 30 aug. 2012, om 22:35 heeft Tony Lindgren het
> >> volgende geschreven:
> >>
> >>> * AnilKumar Ch [120828 01:11]:
> >>>> Adds GPIO pinctrl nodes to am3358_pinmux master node to control
> >>>> user leds (USR0, USR1, USR2 and USR3) present on BeagleBone.
> >>>>
> >>>> Signed-off-by: AnilKumar Ch
> >>>> ---
> >>>> arch/arm/boot/dts/am335x-bone.dts | 14 ++
> >>>> 1 file changed, 14 insertions(+)
> >>>>
> >>>> diff --git a/arch/arm/boot/dts/am335x-bone.dts
> >>>> b/arch/arm/boot/dts/am335x-bone.dts
> >>>> index a7906cb..58f5042 100644
> >>>> --- a/arch/arm/boot/dts/am335x-bone.dts
> >>>> +++ b/arch/arm/boot/dts/am335x-bone.dts
> >>>> @@ -18,6 +18,20 @@
> >>>>reg = <0x8000 0x1000>; /* 256 MB */
> >>>>};
> >>>>
> >>>> +am3358_pinmux: pinmux@44E10800 {
> >>>> +pinctrl-names = "default";
> >>>> +pinctrl-0 = <&userled_pins>;
> >>>> +
> >>>> +userled_pins: pinmux_userled_pins {
> >>>> +pinctrl-single,pins = <
> >>>> +0x54 0x7/* gpmc_a5.gpio1_21, OUTPUT | MODE7 */
> >>>> +0x58 0x17/* gpmc_a6.gpio1_22, OUTPUT_PULLUP | MODE7
> >>>> */
> >>>> +0x5C 0x7/* gpmc_a7.gpio1_23, OUTPUT | MODE7 */
> >>>> +0x60 0x17/* gpmc_a8.gpio1_24, OUTPUT_PULLUP | MODE7
> >>>> */
> >>>> +>;
> >>>> +};
> >>>> +};
> >>>> +
> >>>
> >>> Looks like this patch should also claim these pins by the led driver.
> >>> Then the led driver should just do pinctrl_get_select_default(&pdev->dev)
> >>> in it's probe function to set the pins.
> >>
> >> FWIW, I've been using this for a while now:
> >>
> >> + leds {
> >> + compatible = "gpio-leds";
> >> + heartbeat {
> >> + label = "beaglebone::usr0";
> >> + gpios = <&gpio2 21 0>;
> >> + linux,default-trigger = "heartbeat";
> >> + };
> >> +
> >> + mmc {
> >> + label = "beaglebone:usr1";
> >> + gpios = <&gpio2 22 0>;
> >> + linux,default-trigger = "mmc0";
> >> + };
> >
> > Thanks for the inputs, similar data but not exact is added to
> > v5 series.
> >
> > +gpio-leds {
> > +compatible = "gpio-leds";
> > +pinctrl-names = "default";
> > +pinctrl-0 = <&userled_pins>;
> > +
> > +led0 {
> > +label = "status:green:user0";
> > +gpios = <&gpio2 21 0>;
> > +default-state = "off";
> > +};
> > +
> > +led1 {
> > +label = "status:green:user1";
> > +gpios = <&gpio2 22 0>;
> > +default-state = "off";
> > +};
> > +
> > +led2 {
> > +label = "status:green:user2";
> > +gpios = <&gpio2 23 0>;
> > +default-state = "off";
> > +};
> > +
> > +led3 {
> > +label = "status:green:user3";
> > +gpios = <&gpio2 24 0>;
> > +default-state = "off";
> > +};
> > +};
> > +
> >
> > Can you review my v5 patches and tell me if there are any
> > modifications/changes required?
>
> It would be nice to keep de heartbeat and mmc triggers like we have in the
> 3.2 kernel that ships with the bone
>
I will merge your changes and repost the patches.
Thanks
AnilKumar
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: [PATCH 5/5] of: Modify c_can binding documentation
Hi Marc,
On Fri, Aug 31, 2012 at 14:59:21, AnilKumar, Chimata wrote:
> Modify c_can binding documentation according to recent review comments
> on device tree data addition patches.
>
> Signed-off-by: AnilKumar Ch
> ---
> .../devicetree/bindings/net/can/c_can.txt | 25
>
> 1 file changed, 20 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/net/can/c_can.txt
> b/Documentation/devicetree/bindings/net/can/c_can.txt
> index a43f083..90a70be 100644
> --- a/Documentation/devicetree/bindings/net/can/c_can.txt
> +++ b/Documentation/devicetree/bindings/net/can/c_can.txt
> @@ -8,6 +8,8 @@ Required properties:
> registers map
> - interrupts : property with a value describing the interrupt
> number
> +- status : describes the node status either "disabled" or
> + "okay"
>
> Optional properties:
> - interrupt-parent : The parent interrupt controller
> @@ -20,18 +22,31 @@ Future plan is to migrate hwmod data base contents into
> device tree
> blob so that, all the required data will be used from device tree dts
> file.
>
> -Examples:
> +Example:
>
> - d_can@481D {
> +Step1: SoC common .dtsi file
> +
> + d_can1: d_can@481d {
> compatible = "bosch,d_can";
> - reg = <0x481D 0x1000>;
> - interrupts = <55 0x4>;
> + reg = <0x481d 0x2000>;
> + interrupts = <55>;
> interrupt-parent = <&intc>;
> + status = "disabled";
> };
>
> (or)
>
> - d_can@481D {
> + d_can1: d_can@481d {
> compatible = "bosch,d_can";
> ti,hwmods = "d_can1";
> + reg = <0x481d 0x2000>;
> + interrupts = <55>;
> + interrupt-parent = <&intc>;
> + status = "disabled";
> + };
> +
> +Step 2: board specific .dts file
> +
> + &dcan1 {
> + status = "okay";
> };
> --
> 1.7.9.5
>
>
Can you review this patch and push it to linux-can tree?
Because initial version of this file c_can.txt is in
linux-can tree.
Thanks
AnilKumar
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: [PATCH RESEND v4 2/3] arm/dts: AM33XX: Configure pinmuxs for user leds control on Bone
Hi Koen,
On Fri, Aug 31, 2012 at 21:23:18, Koen Kooi wrote:
>
> Op 30 aug. 2012, om 22:35 heeft Tony Lindgren het volgende
> geschreven:
>
> > * AnilKumar Ch [120828 01:11]:
> >> Adds GPIO pinctrl nodes to am3358_pinmux master node to control
> >> user leds (USR0, USR1, USR2 and USR3) present on BeagleBone.
> >>
> >> Signed-off-by: AnilKumar Ch
> >> ---
> >> arch/arm/boot/dts/am335x-bone.dts | 14 ++
> >> 1 file changed, 14 insertions(+)
> >>
> >> diff --git a/arch/arm/boot/dts/am335x-bone.dts
> >> b/arch/arm/boot/dts/am335x-bone.dts
> >> index a7906cb..58f5042 100644
> >> --- a/arch/arm/boot/dts/am335x-bone.dts
> >> +++ b/arch/arm/boot/dts/am335x-bone.dts
> >> @@ -18,6 +18,20 @@
> >>reg = <0x8000 0x1000>; /* 256 MB */
> >>};
> >>
> >> + am3358_pinmux: pinmux@44E10800 {
> >> + pinctrl-names = "default";
> >> + pinctrl-0 = <&userled_pins>;
> >> +
> >> + userled_pins: pinmux_userled_pins {
> >> + pinctrl-single,pins = <
> >> + 0x54 0x7/* gpmc_a5.gpio1_21, OUTPUT |
> >> MODE7 */
> >> + 0x58 0x17 /* gpmc_a6.gpio1_22,
> >> OUTPUT_PULLUP | MODE7 */
> >> + 0x5C 0x7/* gpmc_a7.gpio1_23, OUTPUT |
> >> MODE7 */
> >> + 0x60 0x17 /* gpmc_a8.gpio1_24,
> >> OUTPUT_PULLUP | MODE7 */
> >> + >;
> >> + };
> >> + };
> >> +
> >
> > Looks like this patch should also claim these pins by the led driver.
> > Then the led driver should just do pinctrl_get_select_default(&pdev->dev)
> > in it's probe function to set the pins.
>
> FWIW, I've been using this for a while now:
>
> + leds {
> + compatible = "gpio-leds";
> + heartbeat {
> + label = "beaglebone::usr0";
> + gpios = <&gpio2 21 0>;
> + linux,default-trigger = "heartbeat";
> + };
> +
> + mmc {
> + label = "beaglebone:usr1";
> + gpios = <&gpio2 22 0>;
> + linux,default-trigger = "mmc0";
> + };
Thanks for the inputs, similar data but not exact is added to
v5 series.
+ gpio-leds {
+ compatible = "gpio-leds";
+ pinctrl-names = "default";
+ pinctrl-0 = <&userled_pins>;
+
+ led0 {
+ label = "status:green:user0";
+ gpios = <&gpio2 21 0>;
+ default-state = "off";
+ };
+
+ led1 {
+ label = "status:green:user1";
+ gpios = <&gpio2 22 0>;
+ default-state = "off";
+ };
+
+ led2 {
+ label = "status:green:user2";
+ gpios = <&gpio2 23 0>;
+ default-state = "off";
+ };
+
+ led3 {
+ label = "status:green:user3";
+ gpios = <&gpio2 24 0>;
+ default-state = "off";
+ };
+ };
+
Can you review my v5 patches and tell me if there are any
modifications/changes required?
Thanks
AnilKumar
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: [PATCH 4/5] leds: leds-gpio: adopt pinctrl support
Hi Tony,
Thanks for the review,
On Fri, Aug 31, 2012 at 21:34:04, Tony Lindgren wrote:
> * AnilKumar Ch [120831 02:30]:
> > Adopt pinctrl support to leds-gpio driver, based on the device
> > pointer (leds-gpio) pinctrl driver configure SoC pins to GPIO
> > mode.
> >
> > Signed-off-by: AnilKumar Ch
> > ---
> > drivers/leds/leds-gpio.c | 31 ---
> > 1 file changed, 24 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/leds/leds-gpio.c b/drivers/leds/leds-gpio.c
> > index c032b21..d98dfb9 100644
> > --- a/drivers/leds/leds-gpio.c
> > +++ b/drivers/leds/leds-gpio.c
> > @@ -20,6 +20,7 @@
> > #include
> > #include
> > #include
> > +#include
> >
> > struct gpio_led_data {
> > struct led_classdev cdev;
> > @@ -236,14 +237,23 @@ static int __devinit gpio_led_probe(struct
> > platform_device *pdev)
> > {
> > struct gpio_led_platform_data *pdata = pdev->dev.platform_data;
> > struct gpio_leds_priv *priv;
> > - int i, ret = 0;
> > + struct pinctrl *pinctrl;
> > + int i = 0;
> > + int ret = 0;
> > +
> > + pinctrl = devm_pinctrl_get_select_default(&pdev->dev);
> > + if (IS_ERR(pinctrl)) {
> > + return PTR_ERR(pinctrl);
> > + }
>
> I think you need to just print out a warning here as most systems don't
> have the pinctrl implemented. And some people just do static pinmuxing
> in the bootloader.
>
Yes, that is better approach I will fix this in my next version.
I will separate out this patch from the series because these changes are
leds-gpio driver specific which are different from DT data (pinctrl and
d_can).
Thanks
AnilKumar
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: [PATCH] arm/dts: AM33XX: Set the default status of module to "disabled" state
Hi Vaibhav,
On Mon, Aug 06, 2012 at 16:59:04, Hiremath, Vaibhav wrote:
> Ideally in common SoC dtsi file we should set all modules
> to "disabled" state and it should get enabled in respective
> EVM/Board dts file as per usage.
>
> This patch sets default status of all modules to "disabled"
> state in am33xx.dtsi file, and as per board requirement, enabled
> in board dts file (like, bone, evm, etc...).
>
> Signed-off-by: Vaibhav Hiremath
> Cc: Benoit Cousson
> Cc: Grant Likely
> Cc: Arnd Bergmann
> CC: Tony Lindgren
> ---
> This patch is tested on BeagleBone platform.
>
> arch/arm/boot/dts/am335x-bone.dts |6 ++
> arch/arm/boot/dts/am335x-evm.dts |6 ++
> arch/arm/boot/dts/am33xx.dtsi |9 +
> 3 files changed, 21 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/boot/dts/am335x-bone.dts
> b/arch/arm/boot/dts/am335x-bone.dts
> index a9af4db..df672b4 100644
> --- a/arch/arm/boot/dts/am335x-bone.dts
> +++ b/arch/arm/boot/dts/am335x-bone.dts
> @@ -17,4 +17,10 @@
> device_type = "memory";
> reg = <0x8000 0x1000>; /* 256 MB */
> };
> +
> + ocp {
> + uart1: serial@44E09000 {
> +status = "okay";
> +};
Minor change, correct indentation here.
> + };
> };
> diff --git a/arch/arm/boot/dts/am335x-evm.dts
> b/arch/arm/boot/dts/am335x-evm.dts
> index d6a97d9..00bbae8 100644
> --- a/arch/arm/boot/dts/am335x-evm.dts
> +++ b/arch/arm/boot/dts/am335x-evm.dts
> @@ -17,4 +17,10 @@
> device_type = "memory";
> reg = <0x8000 0x1000>; /* 256 MB */
> };
> +
> + ocp {
> + uart1: serial@44E09000 {
> +status = "okay";
> +};
ditto
Regards
AnilKumar
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: [PATCH RESEND v2 1/2] arm/dts: Add AM33XX basic pinctrl support
Peter, On Wed, Aug 08, 2012 at 17:35:44, Ujfalusi, Peter wrote: > On 07/31/2012 04:37 PM, AnilKumar, Chimata wrote: > >> I'm just curious about the size here: how it ended up as 0x0338? > >> While looking at the TRM of AM335x I can come up with 0x0238 (0x44e10800 - > >> 0x44e10a38), but I don't see any sings of the remaining 0x0100 to be > >> documented at least. > > > > No, pinmux registers are available till 0x44E10B38, look at AM335x latest > > TRM > > or pinmux utility (we cannot find the exact offsets but pins we can find > > after > > 0x0A38, conf_ddr_resetn) at http://www.ti.com/tool/pinmuxtool > > I have looked at the latest TRM (Rev F, SPRUH73F - public TRM) and there is > not mention of registers between 0x0a34 and 0x0e00. > I can not even find any reference of conf_ddr_resetn register either in the > TRM. > I can not check with the pinmuxtool since it is crashing with wine under > Linux. > There could be registers after 0x0a34, but they are not publicly documented... > Thanks for pointing this, I was referring older version (Rev C) of TRM which have the details which is not present in latest TRM. These pins are dedicated ones so pinmuxing is not required. I will submit new version. Thanks AnilKumar ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: [PATCH v3 1/2] lis3: add generic DT matching code
Hi Mack,
No attachments please.
On Wed, Aug 08, 2012 at 00:19:01, Daniel Mack wrote:
> Hi,
>
> thanks for your review.
>
> On 06.08.2012 12:45, AnilKumar, Chimata wrote:
> > On Sun, Aug 05, 2012 at 21:48:42, Daniel Mack wrote:
> >> On 30.07.2012 09:36, Daniel Mack wrote:
> >>> This patch adds logic to parse lis3 properties from a device tree node
> >>> and store them in a freshly allocated lis3lv02d_platform_data.
> >>>
> >>> Note that the actual match tables are left out here. This part should
> >>> happen in the drivers that bind to the individual busses (SPI/I2C/PCI).
> >>>
> >>> Also adds some DT bindinds documentation.
> >>>
> >>> Signed-off-by: Daniel Mack
> >>> ---
> >>> Changes from v2:
> >>> - kzalloc braino
> >>>
> >>> Changes from v1:
> >>> - some typos in properties fixed
> >>>
> >>>
> >>> Documentation/devicetree/bindings/misc/lis302.txt | 74
> >>> drivers/misc/lis3lv02d/lis3lv02d.c| 137
> >>> ++
> >>> drivers/misc/lis3lv02d/lis3lv02d.h| 4 +
> >>> 3 files changed, 215 insertions(+)
> >>> create mode 100644 Documentation/devicetree/bindings/misc/lis302.txt
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/misc/lis302.txt
> >>> b/Documentation/devicetree/bindings/misc/lis302.txt
> >>> new file mode 100644
> >>> index 000..66230fd
> >>> --- /dev/null
> >>> +++ b/Documentation/devicetree/bindings/misc/lis302.txt
> >>> @@ -0,0 +1,74 @@
> >>> +LIS302 accelerometer devicetree bindings
> >>> +
> >>> +This device is matched via its bus drivers, and has a number of
> >>> properties
> >>> +that apply in on the generic device (independent from the bus).
> >>> +
> >>> +
> >>> +Required properties for the SPI bindings:
> >>> + - compatible: should be set to "st,lis3lv02d_spi"
> >>> + - reg: the chipselect index
> >>> + - spi-max-frequency:maximal bus speed, should be set to 100
> >>> unless
> >>> + constrained by external circuitry
> >>> + - interrupts: the interrupt generated by the device
> >>> +
> >>> +
> >>> +Optional properties for all bus drivers:
> >>> +
> >>> + - st,click-single-{x,y,z}: if present, tells the device to issue an
> >>> + interrupt on single click events on the
> >>> + x/y/z axis.
> >>> + - st,click-double-{x,y,z}: if present, tells the device to issue an
> >>> + interrupt on double click events on the
> >>> + x/y/z axis.
> >>> + - st,click-thresh-{x,y,z}: set the x/y/z axis threshold
> >>> + - st,click-click-time-limit:click time limit, from 0 to 127.5msec
> >>> + with step of 0.5 msec
> >>> + - st,click-latency: click latency, from 0 to 255 msec with
> >>> + step of 1 msec.
> >>> + - st,click-window: click window, from 0 to 255 msec with
> >>> + step of 1 msec.
> >>> + - st,irq{1,2}-disable: disable IRQ 1/2
> >>> + - st,irq{1,2}-ff-wu-1: raise IRQ 1/2 on FF_WU_1 condition
> >>> + - st,irq{1,2}-ff-wu-2: raise IRQ 1/2 on FF_WU_2 condition
> >>> + - st,irq{1,2}-data-ready: raise IRQ 1/2 on data ready contition
> >>> + - st,irq{1,2}-click:raise IRQ 1/2 on click condition
> >>> + - st,irq-open-drain:consider IRQ lines open-drain
> >>> + - st,irq-active-low:make IRQ lines active low
> >>> + - st,wu-duration-1: duration register for Free-Fall/Wake-Up
> >>> + interrupt 1
> >>> + - st,wu-duration-2: duration register for Free-Fall/Wake-Up
> >>> + interrupt 2
> >>> + - st,wakeup-{x,y,z}-{lo,hi}:set wakeup condition on x/y/z axis for
> >>> + upper/lower limit
> >>> + - st,highpass-cutoff-hz=: 1, 2, 4 or 8 for 1Hz, 2Hz, 4Hz or 8Hz of
> >>> + highpass cut-off frequency
> &g
RE: [PATCH v2 1/2] arm/dts: AM33XX: Add D_CAN device tree data
Vaibhav,
Thanks for the review.
On Fri, Aug 03, 2012 at 11:58:41, Hiremath, Vaibhav wrote:
>
>
> On 7/25/2012 5:53 PM, AnilKumar Ch wrote:
> > Add Bosch D_CAN controller device tree data to AM33XX dtsi file
> > by adding d_can device node with all the necessary parameters.
> >
> > Signed-off-by: AnilKumar Ch
> > ---
> > arch/arm/boot/dts/am33xx.dtsi |5 +
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
> > index 9b974dc..2db2ffb 100644
> > --- a/arch/arm/boot/dts/am33xx.dtsi
> > +++ b/arch/arm/boot/dts/am33xx.dtsi
> > @@ -163,5 +163,10 @@
> > #size-cells = <0>;
> > ti,hwmods = "i2c3";
> > };
> > +
> > + dcan1: d_can@481D {
> > + compatible = "bosch,d_can";
> > + ti,hwmods = "d_can1";
> > + };
>
> Anil,
>
> Any reason why we are only specifying dcan1 instance? Shouldn't we
> specify dcan0 as well here?
>
Yes, I agree dcan0 needs to add because SoC support two instances. I
have added d_can1 only because on AM335x-EVM d_can1 is available and
in other case AM335x-bone have two instances available. I will send next
version of these patches.
Regards
AnilKumar
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: [PATCH v4 3/3] can: c_can: Add runtime PM support to Bosch C_CAN/D_CAN controller
Hi Vaibhav, Thanks for the review. On Mon, Aug 06, 2012 at 12:18:32, Hiremath, Vaibhav wrote: > On Sat, Aug 04, 2012 at 00:39:25, Marc Kleine-Budde wrote: > > On 08/03/2012 08:32 AM, Hiremath, Vaibhav wrote: > > > On Thu, Aug 02, 2012 at 18:43:11, AnilKumar, Chimata wrote: > > >> Add Runtime PM support to C_CAN/D_CAN controller. The runtime PM > > >> APIs control clocks for C_CAN/D_CAN IP and prevent access to the > > >> register of C_CAN/D_CAN IP when clock is turned off. > > >> > > >> Signed-off-by: AnilKumar Ch > > >> --- > > >> drivers/net/can/c_can/c_can_platform.c |8 > > >> 1 file changed, 8 insertions(+) > > >> > > >> diff --git a/drivers/net/can/c_can/c_can_platform.c > > >> b/drivers/net/can/c_can/c_can_platform.c > > >> index d0a66cf..83a1e17 100644 > > >> --- a/drivers/net/can/c_can/c_can_platform.c > > >> +++ b/drivers/net/can/c_can/c_can_platform.c > > >> @@ -32,6 +32,7 @@ > > >> #include > > >> #include > > >> #include > > >> +#include > > >> > > >> #include > > >> > > >> @@ -177,6 +178,9 @@ static int __devinit c_can_plat_probe(struct > > >> platform_device *pdev) > > >> goto exit_free_device; > > >> } > > >> > > >> +pm_runtime_enable(&pdev->dev); > > >> +pm_runtime_get_sync(&pdev->dev); > > >> + > > > > > > If module is inserted or built into the kernel, module stays in enabled > > > state always, isn't that wrong? > > > Ideally, you should enable the module when it is required or being used. > > > > Good point. > > > > If you don't access the module's registers in the probe- (or its > > subroutines) it should be enough to enable the module in the open() > > function. Have a look at clk_prepare_enable / clk_disable_unprepare in > > the flexcan driver. > > > > Yeah Marc, something similar, above runtime pm api's should be moved to > open-n-close. > Not all APIs, only *_get_sync and *_put_sync to appropriate positions. I will send next version after fixing this. Regards AnilKumar ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: [PATCH RESEND v2 1/2] arm/dts: Add AM33XX basic pinctrl support
Hi Tony,
On Tue, Aug 07, 2012 at 13:13:48, Tony Lindgren wrote:
> * AnilKumar, Chimata [120731 06:37]:
> > Hi Peter,
> >
> > On Fri, Jul 27, 2012 at 14:40:52, Ujfalusi, Peter wrote:
> > > Hi,
> > >
> > > On 07/24/2012 06:45 PM, AnilKumar Ch wrote:
> > > > Adds basic pinctrl support for AM33XX family of devices. This patch
> > > > is based on the pinctrl-simple driver submitted by Tony Lindgren's
> > > > here: http://lwn.net/Articles/496075/
> > > >
> > > > Signed-off-by: AnilKumar Ch
> > > > ---
> > > > arch/arm/boot/dts/am33xx.dtsi |9 +
> > > > 1 file changed, 9 insertions(+)
> > > >
> > > > diff --git a/arch/arm/boot/dts/am33xx.dtsi
> > > > b/arch/arm/boot/dts/am33xx.dtsi
> > > > index 59509c4..9b974dc 100644
> > > > --- a/arch/arm/boot/dts/am33xx.dtsi
> > > > +++ b/arch/arm/boot/dts/am33xx.dtsi
> > > > @@ -40,6 +40,15 @@
> > > > };
> > > > };
> > > >
> > > > + am3358_pinmux: pinmux@44E10800 {
> > > > + compatible = "pinctrl-single";
> > > > + reg = <0x44E10800 0x0338>;
> > >
> > > I'm just curious about the size here: how it ended up as 0x0338?
> > > While looking at the TRM of AM335x I can come up with 0x0238 (0x44e10800 -
> > > 0x44e10a38), but I don't see any sings of the remaining 0x0100 to be
> > > documented at least.
> >
> > No, pinmux registers are available till 0x44E10B38, look at AM335x latest
> > TRM
> > or pinmux utility (we cannot find the exact offsets but pins we can find
> > after
> > 0x0A38, conf_ddr_resetn) at http://www.ti.com/tool/pinmuxtool
>
> If you have a hole inbetween the registers it sounds like you also have core
> and
> wkup domains? In that case those should be set up as separate controllers as
> other
> SCM registers may be inbetween those domains.
>
In case of AM33XX we have only one padconf domain and it do not have any holes
in between.
Regards
AnilKumar
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: [PATCH v3 1/2] lis3: add generic DT matching code
On Sun, Aug 05, 2012 at 21:48:42, Daniel Mack wrote:
> Ping, anyone?
>
> On 30.07.2012 09:36, Daniel Mack wrote:
> > This patch adds logic to parse lis3 properties from a device tree node
> > and store them in a freshly allocated lis3lv02d_platform_data.
> >
> > Note that the actual match tables are left out here. This part should
> > happen in the drivers that bind to the individual busses (SPI/I2C/PCI).
> >
> > Also adds some DT bindinds documentation.
> >
> > Signed-off-by: Daniel Mack
> > ---
> > Changes from v2:
> > - kzalloc braino
> >
> > Changes from v1:
> > - some typos in properties fixed
> >
> >
> > Documentation/devicetree/bindings/misc/lis302.txt | 74
> > drivers/misc/lis3lv02d/lis3lv02d.c| 137
> > ++
> > drivers/misc/lis3lv02d/lis3lv02d.h| 4 +
> > 3 files changed, 215 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/misc/lis302.txt
> >
> > diff --git a/Documentation/devicetree/bindings/misc/lis302.txt
> > b/Documentation/devicetree/bindings/misc/lis302.txt
> > new file mode 100644
> > index 000..66230fd
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/misc/lis302.txt
> > @@ -0,0 +1,74 @@
> > +LIS302 accelerometer devicetree bindings
> > +
> > +This device is matched via its bus drivers, and has a number of properties
> > +that apply in on the generic device (independent from the bus).
> > +
> > +
> > +Required properties for the SPI bindings:
> > + - compatible: should be set to "st,lis3lv02d_spi"
> > + - reg:the chipselect index
> > + - spi-max-frequency: maximal bus speed, should be set to 100
> > unless
> > + constrained by external circuitry
> > + - interrupts: the interrupt generated by the device
> > +
> > +
> > +Optional properties for all bus drivers:
> > +
> > + - st,click-single-{x,y,z}:if present, tells the device to issue an
> > + interrupt on single click events on the
> > + x/y/z axis.
> > + - st,click-double-{x,y,z}:if present, tells the device to issue an
> > + interrupt on double click events on the
> > + x/y/z axis.
> > + - st,click-thresh-{x,y,z}:set the x/y/z axis threshold
> > + - st,click-click-time-limit: click time limit, from 0 to 127.5msec
> > + with step of 0.5 msec
> > + - st,click-latency: click latency, from 0 to 255 msec with
> > + step of 1 msec.
> > + - st,click-window:click window, from 0 to 255 msec with
> > + step of 1 msec.
> > + - st,irq{1,2}-disable:disable IRQ 1/2
> > + - st,irq{1,2}-ff-wu-1:raise IRQ 1/2 on FF_WU_1 condition
> > + - st,irq{1,2}-ff-wu-2:raise IRQ 1/2 on FF_WU_2 condition
> > + - st,irq{1,2}-data-ready: raise IRQ 1/2 on data ready contition
> > + - st,irq{1,2}-click: raise IRQ 1/2 on click condition
> > + - st,irq-open-drain: consider IRQ lines open-drain
> > + - st,irq-active-low: make IRQ lines active low
> > + - st,wu-duration-1: duration register for Free-Fall/Wake-Up
> > + interrupt 1
> > + - st,wu-duration-2: duration register for Free-Fall/Wake-Up
> > + interrupt 2
> > + - st,wakeup-{x,y,z}-{lo,hi}: set wakeup condition on x/y/z axis for
> > + upper/lower limit
> > + - st,highpass-cutoff-hz=: 1, 2, 4 or 8 for 1Hz, 2Hz, 4Hz or 8Hz of
> > + highpass cut-off frequency
> > + - st,hipass{1,2}-disable: disable highpass 1/2.
> > + - st,default-rate=: set the default rate
> > + - st,axis-{x,y,z}=: set the axis to map to the three
> > coordinates
Some more parameters missing, what about st_min_limits and st_max_limits
required for selftest.
> > +
> > +
> > +Example for a SPI device node:
> > +
> > + lis302@0 {
> > + compatible = "st,lis302dl-spi";
> > + reg = <0>;
> > + spi-max-frequency = <100>;
> > + interrupt-parent = <&gpio>;
> > + interrupts = <104 0>;
> > +
> > + st,click-single-x;
> > + st,click-single-y;
> > + st,click-single-z;
> > + st,click-thresh-x = <10>;
> > + st,click-thresh-y = <10>;
> > + st,click-thresh-z = <10>;
> > + st,irq1-click;
> > + st,irq2-click;
> > + st,wakeup-x-lo;
> > + st,wakeup-x-hi;
> > + st,wakeup-y-lo;
> > + st,wakeup-y-hi;
> > + st,wakeup-z-lo;
> > + st,wakeup-z-hi;
> > + };
Why can't we add these flags in driver itself like below?
Is these parameters varies from SoC to SoC or accelerometer
- to - accelerometer?
#ifdef CONFIG_OF
static stru
RE: [PATCH v4 1/3] can: c_can: Modify c_can device names
On Mon, Aug 06, 2012 at 15:06:49, Marc Kleine-Budde wrote: > On 08/02/2012 03:13 PM, AnilKumar Ch wrote: > > Modify c_can device names from *_CAN_DEVTYPE to BOSCH_*_CAN to make > > use of same names for array indexes in c_can_id_table[] as well as > > device names. > > > > This patch also add indexes to c_can_id_table array. > > > > Signed-off-by: AnilKumar Ch > > I'm taking patch 1 and 2. Runtime pm support seems to need discussion. > > Applied to can-next/master > Thanks Marc, I will update on runtime PM functionality. Regards AnilKumar ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: [PATCH v3 1/3] can: c_can: Add device tree support to Bosch C_CAN/D_CAN controller
Hi Arnd, Thanks for the review On Thu, Aug 02, 2012 at 17:03:59, Arnd Bergmann wrote: > On Thursday 02 August 2012 16:32:17 AnilKumar Ch wrote: > > +- interrupt-parent : The parent interrupt controller > > + > > +Optional properties: > > +- ti,hwmods: Must be "d_can" or "c_can", n being the > > + instance number > > interrupt-parent should be optional, not mandatory. It's usually implied > by the parent's interrupt-parent. > In case of AM33XX specific we need to connect interrupt to interrupt controller so I added to mandatory list. Yes I agree with you, in generic case this should be optional I will change. Thanks AnilKumar ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: [PATCH v3 0/3] Add DT support to C_CAN/D_CAN controller
Marc,
On Thu, Aug 02, 2012 at 16:53:38, Marc Kleine-Budde wrote:
> On 08/02/2012 01:21 PM, AnilKumar, Chimata wrote:
> > Marc,
> >
> > On Thu, Aug 02, 2012 at 16:43:04, Marc Kleine-Budde wrote:
> >> On 08/02/2012 01:02 PM, AnilKumar Ch wrote:
> >>> This patch series adds the device tree support and Runtime PM support
> >>> to C_CAN/D_CAN controller.
> >>>
> >>> These patches have been tested on AM335x EVM using some additional
> >>> patches to add device tree data to EVM dts files and to initialize
> >>> D_CAN RAM. D_CAN raminit is controlled from control module register.
> >>> This patch will be submitted once control module MFD driver support
> >>> is added.
> >>>
> >>> These patches are based on linx-can-next tree.
> >>>
> >>> Due to lack of hardware I am not able to test c_can functionality.
> >>> I appreciate if anyone can test c_can functionality with this patch
> >>> series.
> >>>
> >>> Changes from v2:
> >>> - Incorporated Marcs on v2
> >>> * Fix compilation errors in pci due to device name changes
> >>> in v2 by adding new patch.
> >>>
> >>> Changes from v1:
> >>> - Separated 4 patches into CAN driver specific and device
> >>> tree data addition specific.
> >>> - Incorporated Marc's comments on v1
> >>> * Modified c_can_dev_id enum to handle both devtype and
> >>> platform device id index.
> >>> * Removed "legacy bosch,c_can_platform" from DT bindings
> >>>
> >>> AnilKumar Ch (3):
> >>> can: c_can: Add device tree support to Bosch C_CAN/D_CAN controller
> >>> can: c_can: Modify c_can device names in c_can_pci driver
> >>
> >> You break bisectability here. After patch 1 the pci driver will not
> >> compile anymore. I suggest to do the renaming of enum c_can_dev_id and
> >> all it's users in patch 1.
> >>
> >
> > I will merge patch 1 and 2 and submit v4.
>
> But changing the pci driver has nothing to do with the subject ("Add
> device tree support to Bosch C_CAN/D_CAN controller").
>
> It's considered bad practise to do so.
>
In that case I will change the patch 1 according to "C_CAN_PLTFORM_DEVTYPE"
and flag name changes to BOSCH_* will be in patch 2. So that we can get rid
of these two problems.
Regards
AnilKumar
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: [PATCH v3 0/3] Add DT support to C_CAN/D_CAN controller
Marc, On Thu, Aug 02, 2012 at 16:43:04, Marc Kleine-Budde wrote: > On 08/02/2012 01:02 PM, AnilKumar Ch wrote: > > This patch series adds the device tree support and Runtime PM support > > to C_CAN/D_CAN controller. > > > > These patches have been tested on AM335x EVM using some additional > > patches to add device tree data to EVM dts files and to initialize > > D_CAN RAM. D_CAN raminit is controlled from control module register. > > This patch will be submitted once control module MFD driver support > > is added. > > > > These patches are based on linx-can-next tree. > > > > Due to lack of hardware I am not able to test c_can functionality. > > I appreciate if anyone can test c_can functionality with this patch > > series. > > > > Changes from v2: > > - Incorporated Marcs on v2 > > * Fix compilation errors in pci due to device name changes > > in v2 by adding new patch. > > > > Changes from v1: > > - Separated 4 patches into CAN driver specific and device > > tree data addition specific. > > - Incorporated Marc's comments on v1 > > * Modified c_can_dev_id enum to handle both devtype and > > platform device id index. > > * Removed "legacy bosch,c_can_platform" from DT bindings > > > > AnilKumar Ch (3): > > can: c_can: Add device tree support to Bosch C_CAN/D_CAN controller > > can: c_can: Modify c_can device names in c_can_pci driver > > You break bisectability here. After patch 1 the pci driver will not > compile anymore. I suggest to do the renaming of enum c_can_dev_id and > all it's users in patch 1. > I will merge patch 1 and 2 and submit v4. Regards AnilKumar ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: [PATCH v2 1/2] can: c_can: Add device tree support to Bosch C_CAN/D_CAN controller
Hi Marc, On Thu, Aug 02, 2012 at 13:29:44, Marc Kleine-Budde wrote: > On 07/25/2012 04:12 PM, AnilKumar, Chimata wrote: > > Marc, > > > > On Wed, Jul 25, 2012 at 19:17:52, Marc Kleine-Budde wrote: > >> On 07/25/2012 02:18 PM, AnilKumar Ch wrote: > >>> Add device tree support to C_CAN/D_CAN controller and usage details > >>> are added to device tree documentation. Driver was tested on AM335x > >>> EVM. > >> > >> Does not apply to linux-can-next, as Viresh Kumar's patch "net/c_can: > >> remove conditional compilation of clk code" is not yet included. I > >> suggest to delay this patch until we have Viresh's patch in net-next. > >> > >> See comment inline. > > > > Ok, I will wait till net-next is updated with Viresh Kumar's patch. > > Now Viresh's patch is in net-next, feel free to post your new patches. I will send the patches. Thanks AnilKumar ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: [PATCH RESEND v2 1/2] arm/dts: Add AM33XX basic pinctrl support
Hi Peter,
On Fri, Jul 27, 2012 at 14:40:52, Ujfalusi, Peter wrote:
> Hi,
>
> On 07/24/2012 06:45 PM, AnilKumar Ch wrote:
> > Adds basic pinctrl support for AM33XX family of devices. This patch
> > is based on the pinctrl-simple driver submitted by Tony Lindgren's
> > here: http://lwn.net/Articles/496075/
> >
> > Signed-off-by: AnilKumar Ch
> > ---
> > arch/arm/boot/dts/am33xx.dtsi |9 +
> > 1 file changed, 9 insertions(+)
> >
> > diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
> > index 59509c4..9b974dc 100644
> > --- a/arch/arm/boot/dts/am33xx.dtsi
> > +++ b/arch/arm/boot/dts/am33xx.dtsi
> > @@ -40,6 +40,15 @@
> > };
> > };
> >
> > + am3358_pinmux: pinmux@44E10800 {
> > + compatible = "pinctrl-single";
> > + reg = <0x44E10800 0x0338>;
>
> I'm just curious about the size here: how it ended up as 0x0338?
> While looking at the TRM of AM335x I can come up with 0x0238 (0x44e10800 -
> 0x44e10a38), but I don't see any sings of the remaining 0x0100 to be
> documented at least.
No, pinmux registers are available till 0x44E10B38, look at AM335x latest TRM
or pinmux utility (we cannot find the exact offsets but pins we can find after
0x0A38, conf_ddr_resetn) at http://www.ti.com/tool/pinmuxtool
Regards
AnilKumar
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: [PATCH v2 1/2] arm/dts: AM33XX: Add D_CAN device tree data
On Thu, Jul 26, 2012 at 14:16:33, Koen Kooi wrote:
>
> Op 25 jul. 2012, om 14:23 heeft AnilKumar Ch het volgende
> geschreven:
>
> > Add Bosch D_CAN controller device tree data to AM33XX dtsi file
> > by adding d_can device node with all the necessary parameters.
> >
> > Signed-off-by: AnilKumar Ch
> > ---
> > arch/arm/boot/dts/am33xx.dtsi |5 +
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
> > index 9b974dc..2db2ffb 100644
> > --- a/arch/arm/boot/dts/am33xx.dtsi
> > +++ b/arch/arm/boot/dts/am33xx.dtsi
> > @@ -163,5 +163,10 @@
> > #size-cells = <0>;
> > ti,hwmods = "i2c3";
> > };
> > +
> > + dcan1: d_can@481D {
> > + compatible = "bosch,d_can";
> > + ti,hwmods = "d_can1";
> > + };
> > };
>
> I scanned the linux-networking mailinglist and l-o-ml, but I can't find the
> patchset that actually adds the d_can drivers, could you provide a link to
> that? I have 2 different CAN capes I'd like to test on beaglebone.
>
You can find it from linux-next or net-next trees, D_CAN support is added
to C_CAN driver.
http://git.kernel.org/?p=linux/kernel/git/davem/net-next.git;
a=commitdiff;h=69927fccd96b15bd228bb82d356a7a2a0cfaeefb
Regards
AnilKumar
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: [PATCH v2 1/2] can: c_can: Add device tree support to Bosch C_CAN/D_CAN controller
Marc,
On Wed, Jul 25, 2012 at 19:17:52, Marc Kleine-Budde wrote:
> On 07/25/2012 02:18 PM, AnilKumar Ch wrote:
> > Add device tree support to C_CAN/D_CAN controller and usage details
> > are added to device tree documentation. Driver was tested on AM335x
> > EVM.
>
> Does not apply to linux-can-next, as Viresh Kumar's patch "net/c_can:
> remove conditional compilation of clk code" is not yet included. I
> suggest to delay this patch until we have Viresh's patch in net-next.
>
> See comment inline.
Ok, I will wait till net-next is updated with Viresh Kumar's patch.
>
> > Signed-off-by: AnilKumar Ch
> > ---
> > .../devicetree/bindings/net/can/c_can.txt | 37 +
> > drivers/net/can/c_can/c_can.h |5 +-
> > drivers/net/can/c_can/c_can_platform.c | 57
> > ++--
> > 3 files changed, 80 insertions(+), 19 deletions(-)
> > create mode 100644 Documentation/devicetree/bindings/net/can/c_can.txt
> >
> > diff --git a/Documentation/devicetree/bindings/net/can/c_can.txt
> > b/Documentation/devicetree/bindings/net/can/c_can.txt
> > new file mode 100644
> > index 000..dc4aec5
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/can/c_can.txt
> > @@ -0,0 +1,37 @@
> > +Bosch C_CAN/D_CAN controller Device Tree Bindings
> > +-
> > +
> > +Required properties:
> > +- compatible : Should be "bosch,c_can" for C_CAN controllers
> > and
> > + "bosch,d_can" for D_CAN controllers.
> > +- reg : physical base address and size of the
> > C_CAN/D_CAN
> > + registers map
> > +- interrupts : property with a value describing the interrupt
> > + number
> > +- interrupt-parent : The parent interrupt controller
> > +
> > +Optional properties:
> > +- ti,hwmods: Must be "d_can" or "c_can", n being the
> > + instance number
> > +
> > +Note: "ti,hwmods" field is used to fetch the base address and irq
> > +resources from TI, omap hwmod data base during device registration.
> > +Future plan is to migrate hwmod data base contents into device tree
> > +blob so that, all the required data will be used from device tree dts
> > +file.
> > +
> > +Examples:
> > +
> > + d_can@481D {
> > + compatible = "bosch,d_can";
> > + reg = <0x481D 0x1000>;
> > + interrupts = <55 0x4>;
> > + interrupt-parent = <&intc>;
> > + };
> > +
> > +(or)
> > +
> > + d_can@481D {
> > + compatible = "bosch,d_can";
> > + ti,hwmods = "d_can1";
> > + };
> > diff --git a/drivers/net/can/c_can/c_can.h b/drivers/net/can/c_can/c_can.h
> > index 01a7049..4e56baa 100644
> > --- a/drivers/net/can/c_can/c_can.h
> > +++ b/drivers/net/can/c_can/c_can.h
> > @@ -143,8 +143,9 @@ static const u16 reg_map_d_can[] = {
> > };
> >
> > enum c_can_dev_id {
> > - C_CAN_DEVTYPE,
> > - D_CAN_DEVTYPE,
> > + BOSCH_C_CAN_PLATFORM,
> > + BOSCH_C_CAN,
> > + BOSCH_D_CAN,
>
> Note: these symbols are used in "drivers/net/can/c_can/c_can_pci.c", too.
>
Oops! I missed out. Separate patch will be added in v3 with this series to take
care of this issue.
Regards
AnilKumar
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: [PATCH v2 2/2] arm/dts: Configure pinmuxs for user leds control on Bone
Hi Tony,
Thanks for the review.
On Tue, Jul 24, 2012 at 14:00:08, Tony Lindgren wrote:
> * AnilKumar Ch [120720 00:36]:
> > Adds GPIO pinctrl nodes to am3358_pinmux master node to control
> > user leds (USR0, USR1, USR2 and USR3) present on BeagleBone.
> >
> > Signed-off-by: AnilKumar Ch
> > ---
> > arch/arm/boot/dts/am335x-bone.dts | 15 +++
> > 1 files changed, 15 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/arm/boot/dts/am335x-bone.dts
> > b/arch/arm/boot/dts/am335x-bone.dts
> > index a4d4415..452ce3f 100644
> > --- a/arch/arm/boot/dts/am335x-bone.dts
> > +++ b/arch/arm/boot/dts/am335x-bone.dts
> > @@ -28,3 +28,18 @@
> > };
> >
> > /include/ "tps65217.dtsi"
> > +
> > +&am3358_pinmux {
> > +
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&userled_pins>;
> > +
> > + userled_pins: pinmux_userled_pins {
> > + pinctrl-single,pins = <
> > + 0x54 0x7/* gpmc_a5.gpio1_21, OMAP_PIN_OUTPUT |
> > OMAP_MUX_MODE7 */
> > + 0x58 0x17 /* gpmc_a6.gpio1_22,
> > OMAP_PIN_OUTPUT_PULLUP | OMAP_MUX_MODE7 */
> > + 0x5C 0x7/* gpmc_a7.gpio1_23, OMAP_PIN_OUTPUT |
> > OMAP_MUX_MODE7 */
> > + 0x60 0x17 /* gpmc_a8.gpio1_24,
> > OMAP_PIN_OUTPUT_PULLUP | OMAP_MUX_MODE7 */
> > + >;
> > + };
> > +};
>
> Let's standardize on the following minimal commenting as those can be
> search and replaced the same way when we have preprocessing available:
>
> pinctrl-single,pins = <
> 0x54 0x7/* gpmc_a5.gpio1_21 OUTPUT | MODE7 */
> 0x58 0x17 /* gpmc_a6.gpio1_22, OUTPUT_PULLUP |
> MODE7 */
> 0x5C 0x7/* gpmc_a7.gpio1_23, OUTPUT | MODE7 */
> 0x60 0x17 /* gpmc_a8.gpio1_24, OUTPUT_PULLUP |
> MODE7 */
> >;
>
Point taken, I thought the same after v2 submission. I will send the updated
version with this fix.
Thanks
AnilKumar
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: [PATCH v2 1/2] arm/dts: Add AM33XX basic pinctrl support
On Tue, Jul 24, 2012 at 14:02:04, Tony Lindgren wrote:
> * AnilKumar Ch [120720 00:36]:
> > Add basic pinctrl support for AM33XX family of devices by adding DT
> > data to am33xx dtsi file. These patches are based on pinctrl-single
> > driver and tested on am335x-evm & am335x-bone devices.
> >
> > Signed-off-by: AnilKumar Ch
> > ---
> > arch/arm/boot/dts/am33xx.dtsi |9 +
> > 1 files changed, 9 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
> > index 59509c4..9b974dc 100644
> > --- a/arch/arm/boot/dts/am33xx.dtsi
> > +++ b/arch/arm/boot/dts/am33xx.dtsi
> > @@ -40,6 +40,15 @@
> > };
> > };
> >
> > + am3358_pinmux: pinmux@44E10800 {
> > + compatible = "pinctrl-single";
> > + reg = <0x44E10800 0x0338>;
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + pinctrl-single,register-width = <32>;
> > + pinctrl-single,function-mask = <0x7F>;
> > + };
> > +
> > /*
> > * XXX: Use a flat representation of the AM33XX interconnect.
> > * The real AM33XX interconnect network is quite complex.Since
>
> Is there only one padconf domain on 33xx instead of separate core
> and wkup domains like omap3 and omap4 have?
>
Yes, in case of AM33XX we have only on padconf domain.
Regards
AnilKumar
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: [PATCH v2 1/3] regulator: tps65217: Add device tree support
+Tony Tony, On Fri, Jul 13, 2012 at 15:24:54, Mark Brown wrote: > On Fri, Jul 13, 2012 at 05:43:30AM +0000, AnilKumar, Chimata wrote: > > > Thanks much, are you going to push reset of the patches in this series? > > No, there's no dependency so I'd expect them to be applied by the > architecture maintainers. > Could you please push these [2/3 and 3/3] patches to linux-omap "devel-dt tree"? http://marc.info/?l=linux-omap&m=134191889501150&w=2 http://marc.info/?l=linux-omap&m=134191891701159&w=2 After these patches, "PMIC I2C slave address addition for AM335x EVM/Bone will be submitted, which actually passes DT data to tps65910/217 drivers" Regards AnilKumar ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: regulator: tps65217: Add device tree support
+cc Mark
Hi Dan,
Thanks for checking the static code warnings.
On Fri, Jul 13, 2012 at 14:32:20, Dan Carpenter wrote:
> Hello AnilKumar Ch,
>
> This is a semi-automatic email about new static checker warnings.
>
> The patch a7f1b63eb856: "regulator: tps65217: Add device tree
> support" from Jul 10, 2012, leads to the following Smatch complaint:
>
> drivers/mfd/tps65217.c:245 tps65217_probe()
>error: we previously assumed 'pdata' could be null (see line 205)
>
> drivers/mfd/tps65217.c
>204
>205if (!pdata && client->dev.of_node)
> ^^
> New check.
!pdata in the above if condition is not required if we do a separate check for
"pdata == NULL". So removing pdata check from this and adding a separate
check for NULL pointer.
>
>206pdata = tps65217_parse_dt(client);
Above two lines will be replaced with these lines
if (client->dev.of_node)
pdata = tps65217_parse_dt(client);
if (!pdata) {
dev_err(&client->dev, "tps65217 requires
platform data\n");
return -EINVAL;
}
I will submit a separate patch for this.
Thanks
AnilKumar
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: [PATCH v2 1/3] regulator: tps65217: Add device tree support
Hi Mark, On Thu, Jul 12, 2012 at 22:58:37, Mark Brown wrote: > On Tue, Jul 10, 2012 at 04:39:42PM +0530, AnilKumar Ch wrote: > > This commit adds device tree support for tps65217 pmic. And usage > > details are added to device tree documentation. Driver is tested > > by using kernel module with regulator set and get APIs. > > Applied, thanks. Thanks much, are you going to push reset of the patches in this series? Thanks AnilKumar ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: [PATCH 1/2] arm/dts: Add AM33XX basic pinctrl support
Hi Tony,
Thanks for reviewing.
On Fri, Jul 06, 2012 at 13:53:44, Tony Lindgren wrote:
> * AnilKumar Ch [120705 02:18]:
> > Adds basic pinctrl support for AM33XX family of devices. This patch
> > is based on the pinctrl-simple driver submitted by Tony Lindgren's
> > here: http://lwn.net/Articles/496075/
> >
> > Signed-off-by: AnilKumar Ch
> > ---
> > arch/arm/boot/dts/am33xx.dtsi | 12
> > 1 file changed, 12 insertions(+)
> >
> > diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
> > index 59509c4..85def31 100644
> > --- a/arch/arm/boot/dts/am33xx.dtsi
> > +++ b/arch/arm/boot/dts/am33xx.dtsi
> > @@ -40,6 +40,18 @@
> > };
> > };
> >
> > + am3358_pinmux: pinmux@44E10800 {
> > + compatible = "ti,omap4-padconf";
> > + reg = <0x44E10800 0x0338>;
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + #pinctrl-cells = <2>;
> > + pinctrl-simple,register-width = <32>;
> > + pinctrl-simple,function-mask = <0x7>;
> > + pinctrl-simple,function-off = <0x>;
> > + pinctrl-simple,pinconf-mask = <0x78>;
> > + };
> > +
>
> You might want to update to the latest version, which is now called
> pinctrl-single instead of pinctrl-simple. Should be easy to update,
> note that we're still waiting on people to comment on the binding,
> so you might want to wait a bit before reposting so we have the
> driver merged.
In that case, I will submit the patches with respect to pinctrl-single
after it is available in linux-next tree.
Thanks
AnilKumar
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: [PATCH 1/3] regulator: tps65217: Add device tree support
Hi Mark, Thanks for quick reply. On Mon, Jun 25, 2012 at 17:42:20, Mark Brown wrote: > On Mon, Jun 25, 2012 at 05:34:36PM +0530, AnilKumar Ch wrote: > > This commit adds device tree support for tps65217 pmic. And usage > > details are added to device tree documentation. > > > > Driver is tested by using kernel module with regulator set and get > > APIs. > > This needs to be updated for the patches Laxman recently posted for the > regulator-compatible property (which are currently blocked on review > from the ST Ericcsson guys but I'll probably apply them in the next > couple of days if I don't hear back from them). > In that case, I will resubmit the patches once "regulator-compatible property" patches are pushed to "linux-next". Thanks AnilKumar ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
