From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752793AbbHUDWX (ORCPT ); Thu, 20 Aug 2015 23:22:23 -0400 Received: from mail-by2on0142.outbound.protection.outlook.com ([207.46.100.142]:22916 "EHLO na01-by2-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751658AbbHUDWW (ORCPT ); Thu, 20 Aug 2015 23:22:22 -0400 From: Wang Dongsheng To: Linus Walleij , John Stultz , Russell King - ARM Linux CC: Alessandro Zummo , Alexandre Belloni , Shawn Guo , "Nair, Sandeep" , Hans de Goede , "Huan Wang" , "linux-arm-kernel@lists.infradead.org" , "linux-kernel@vger.kernel.org" , "rtc-linux@googlegroups.com" Subject: RE: [PATCH 2/2] soc/fsl: add ftm alarm driver for ls1021a platform Thread-Topic: [PATCH 2/2] soc/fsl: add ftm alarm driver for ls1021a platform Thread-Index: AQHQ1MP4f7feO5asC02gkGSO02rhIp4J9ZAAgADaepCAAHg4AIAKiIxQ Date: Fri, 21 Aug 2015 03:07:46 +0000 Message-ID: References: <1439358807-9024-1-git-send-email-dongsheng.wang@freescale.com> <1439358807-9024-2-git-send-email-dongsheng.wang@freescale.com> In-Reply-To: Accept-Language: zh-CN, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: spf=none (sender IP is ) smtp.mailfrom=Dongsheng.Wang@freescale.com; x-originating-ip: [192.158.241.86] x-microsoft-exchange-diagnostics: 1;BY1PR0301MB0870;5:g1wvlrbbnZHU3Y3FdDDP6uEBtH6luxB/7qE10ThmN4bzKDy90KxFTfheUkVc+87v6extDfDiepVM+QuCDLnqxYcOxfaBvM3AKOfr/kPg35PYEDP9NwYDUiyO7oYykgNU3WZ36xnqvCKAeJM8l2lPsg==;24:bJfqHQ7kFW/JPJPGqDFm9hjfeTFAflVkSCAguEGsVCcKxuzjU4XpqNcBtgB/n5QCEVJZQC0vbR7mRcu0enJVx2gbinbUZoVMLGHT3OCSD6U=;20:9oJ2wj5dqYZAgTIeY2VnwR8LSvsM8hWnR8DtGDi+bBosg5PhfXrfMkAn2kBXzxaNu3Hi5ng6MPdo1eUPbnM3Mw== x-microsoft-antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:BY1PR0301MB0870; x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:; x-exchange-antispam-report-cfa-test: BCL:0;PCL:0;RULEID:(601004)(8121501046)(5005006)(3002001);SRVR:BY1PR0301MB0870;BCL:0;PCL:0;RULEID:;SRVR:BY1PR0301MB0870; x-forefront-prvs: 067553F396 x-forefront-antispam-report: SFV:NSPM;SFS:(10019020)(6009001)(13464003)(199003)(189002)(24454002)(377454003)(575784001)(62966003)(50986999)(54356999)(87936001)(77156002)(46102003)(19580395003)(2656002)(101416001)(76176999)(122556002)(19580405001)(86362001)(40100003)(105586002)(106116001)(106356001)(102836002)(77096005)(5002640100001)(5003600100002)(99286002)(2950100001)(64706001)(81156007)(66066001)(97736004)(92566002)(76576001)(68736005)(4001540100001)(5001830100001)(5001770100001)(74316001)(5001860100001)(2900100001)(5001920100001)(10400500002)(5007970100001)(189998001)(33656002)(93886004)(5004730100002)(5001960100002);DIR:OUT;SFP:1102;SCL:1;SRVR:BY1PR0301MB0870;H:SN1PR0301MB1616.namprd03.prod.outlook.com;FPR:;SPF:None;PTR:InfoNoRecords;A:1;MX:1;LANG:en; Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 X-OriginatorOrg: freescale.com X-MS-Exchange-CrossTenant-originalarrivaltime: 21 Aug 2015 03:07:46.1563 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 710a03f5-10f6-4d38-9ff4-a80b81da590d X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY1PR0301MB0870 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from base64 to 8bit by mail.home.local id t7L3MUDH031359 Hi Walleij and Russell, I will drop this patch. Thanks for your review. [PATCH v2 1/2] soc/fsl: add freescale dir for SOC specific drivers But the 1/2 of the patches also need, because there has another patch(Freescale FPGA driver) need 1/2 patch. Need I push the 1/2 patch with another patches(Freescale FPGA driver) or push 1/2 standalone without another patch? Regards, -Dongsheng > -----Original Message----- > From: Linus Walleij [mailto:linus.walleij@linaro.org] > Sent: Friday, August 14, 2015 6:07 PM > To: Wang Dongsheng-B40534; John Stultz > Cc: Alessandro Zummo; Alexandre Belloni; Shawn Guo; Nair, Sandeep; Hans de Goede; > Wang Huan-B18965; linux-arm-kernel@lists.infradead.org; linux- > kernel@vger.kernel.org; rtc-linux@googlegroups.com > Subject: Re: [PATCH 2/2] soc/fsl: add ftm alarm driver for ls1021a platform > > On Fri, Aug 14, 2015 at 5:12 AM, Wang Dongsheng > wrote: > >> On Wed, Aug 12, 2015 at 7:53 AM, Dongsheng Wang > >> wrote: > >> > >> > From: Wang Dongsheng > >> > > >> > Only Ftm0 can be used when system going to deep sleep. So this driver > >> > to support ftm0 as a wakeup source. > >> > > >> > Signed-off-by: Wang Dongsheng > >> > --- > >> > *V2* > >> > Change Copyright 2014 to 2015. > >> (...) > >> > +config FTM_ALARM > >> > + bool "FTM alarm driver" > >> > + depends on SOC_LS1021A > >> > + default n > >> > + help > >> > + Say y here to enable FTM alarm support. The FTM alarm provides > >> > + alarm functions for wakeup system from deep sleep. There is only > >> > + one FTM can be used in ALARM(FTM 0). > >> (...) > >> > +static u32 time_to_cycle(unsigned long time) > >> > +static u32 cycle_to_time(u32 cycle) > >> > +static int ftm_set_alarm(u64 cycle) > >> > +static irqreturn_t ftm_alarm_interrupt(int irq, void *dev_id) > >> > +static ssize_t ftm_alarm_show(struct device *dev, > >> > + struct device_attribute *attr, > >> > + char *buf) > >> > +static ssize_t ftm_alarm_store(struct device *dev, > >> > + struct device_attribute *attr, > >> > + const char *buf, size_t count) > >> (...) > >> > +static struct device_attribute ftm_alarm_attributes = __ATTR(ftm_alarm, > 0644, > >> > + ftm_alarm_show, ftm_alarm_store); > >> > >> If you're gonna invent ABIs, document then in Documentation/ABI/testing/*. > >> > >> But I don't get it. Why is this driver not in drivers/rtc? > >> > >> It does a subset of what an RTC does. The ioctl()'s of an RTC > >> can do what you want to do. And much much more. > >> > >> If it can't do all an RTC can do, surely the RTC subsystem > >> can be augmented to host it anyway. It's way to close to > >> an RTC to have it's own random sysfs driver like this. > >> > >> Unless I'm totally off, rewrite this to an RTC driver and post > >> it to the RTC maintainers. > > > > FlexTimer is not a RTC device and not have any rtc deivce function. They > belong to > > different devices, why we need to register this to RTC framework? I am > confused about this. > > > > Now in freescale layerscape platform this driver is only for FlexTimer0, and > not > > fit for each flextimer. Because only FlexTimer0 still turn-on when system in > the Deep Sleep. > > > > If the "alarm" make you feel confused or mislead you think this is a RTC > devices. I think > > I need to change the "alarm" to "timer". > > I think it is an RTC, it is just that the hardware engineer > designed it with a wakeup usecase in mind and did not call > it an RTC. Wakeup is one of the things RTCs do. > > If you inspect a few drivers in drivers/rtc such as drivers/rtc/rtc-pl030.c > you will find that they are just as crude as this "alarm" thing. > > It has a counter that counts cycles, it has a comparator > and an alarm function. It is an on-chip RTC, just like PL030 > no matter what the datasheet or hardware engineer thinks it > should be called, the Linux kernel calls this an RTC, and it > has a subsystem for handling it, so we should use it and > not invent random new stuff. > > If the hardware is really so strange that the counter can only > be started if you also put an alarm at the same time (I doubt > it, but OK if you say so) it is a subset of an RTC that can > only be used for alarms but not timekeeping, but it should > *still* live in drivers/rtc. > > Think for a moment on the huge effort that John Stultz put into > integrating Android alarm timers with POSIX and the RTC > subsystem and fixing it all from the smallest handset to > the largest S360 supercomputer. The approach of a custom > device just throws all of that out the window and reinvents the > mechanism in userspace, forcing all standardized userspace to > have special code to handle this special alarm with its > special sysfs ABI. > > Check > commit ff3ead96d17f47ee70c294a5cc2cce9b61e82f0f > "timers: Introduce in-kernel alarm-timer interface" > for example. > > Even if you persist on keeping it in its own magic driver > like this, it should implement the alarm timer interface > from and I bet after that you don't > need the sysfs files anymore, as the system will sleep > and wake up from the regular syscalls instead of using > magic poking in sysfs from userspace. AFAICT this hardware > is designed for exactly this usecase. > > tools/testing/selftests/timers/alarmtimer-suspend.c > is there for you to test your driver with alarmtimer > support. > > Needless to say: if you implement it as an RTC you get the > alarmtimer interaction for free. That is why we have the > subsystem after all: to be helpful. > > Yours, > Linus Walleij {.n++%ݶw{.n+{G{ayʇڙ,jfhz_(階ݢj"mG?&~iOzv^m ?I From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from na01-bn1-obe.outbound.protection.outlook.com (mail-bn1on0133.outbound.protection.outlook.com. [157.56.110.133]) by gmr-mx.google.com with ESMTPS id g144si674064ywb.6.2015.08.20.20.07.49 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Thu, 20 Aug 2015 20:07:50 -0700 (PDT) From: Wang Dongsheng To: Linus Walleij , John Stultz , Russell King - ARM Linux CC: Alessandro Zummo , Alexandre Belloni , Shawn Guo , "Nair, Sandeep" , Hans de Goede , "Huan Wang" , "linux-arm-kernel@lists.infradead.org" , "linux-kernel@vger.kernel.org" , "rtc-linux@googlegroups.com" Subject: [rtc-linux] RE: [PATCH 2/2] soc/fsl: add ftm alarm driver for ls1021a platform Date: Fri, 21 Aug 2015 03:07:46 +0000 Message-ID: References: <1439358807-9024-1-git-send-email-dongsheng.wang@freescale.com> <1439358807-9024-2-git-send-email-dongsheng.wang@freescale.com> In-Reply-To: Content-Type: text/plain; charset=UTF-8 MIME-Version: 1.0 Reply-To: rtc-linux@googlegroups.com List-ID: List-Post: , List-Help: , List-Archive: , List-Unsubscribe: , Hi Walleij and Russell, I will drop this patch. Thanks for your review. [PATCH v2 1/2] soc/fsl: add freescale dir for SOC specific drivers But the 1/2 of the patches also need, because there has another patch(Freescale FPGA driver) need 1/2 patch. Need I push the 1/2 patch with another patches(Freescale FPGA driver) or push 1/2 standalone without another patch? Regards, -Dongsheng > -----Original Message----- > From: Linus Walleij [mailto:linus.walleij@linaro.org] > Sent: Friday, August 14, 2015 6:07 PM > To: Wang Dongsheng-B40534; John Stultz > Cc: Alessandro Zummo; Alexandre Belloni; Shawn Guo; Nair, Sandeep; Hans de Goede; > Wang Huan-B18965; linux-arm-kernel@lists.infradead.org; linux- > kernel@vger.kernel.org; rtc-linux@googlegroups.com > Subject: Re: [PATCH 2/2] soc/fsl: add ftm alarm driver for ls1021a platform > > On Fri, Aug 14, 2015 at 5:12 AM, Wang Dongsheng > wrote: > >> On Wed, Aug 12, 2015 at 7:53 AM, Dongsheng Wang > >> wrote: > >> > >> > From: Wang Dongsheng > >> > > >> > Only Ftm0 can be used when system going to deep sleep. So this driver > >> > to support ftm0 as a wakeup source. > >> > > >> > Signed-off-by: Wang Dongsheng > >> > --- > >> > *V2* > >> > Change Copyright 2014 to 2015. > >> (...) > >> > +config FTM_ALARM > >> > + bool "FTM alarm driver" > >> > + depends on SOC_LS1021A > >> > + default n > >> > + help > >> > + Say y here to enable FTM alarm support. The FTM alarm provides > >> > + alarm functions for wakeup system from deep sleep. There is only > >> > + one FTM can be used in ALARM(FTM 0). > >> (...) > >> > +static u32 time_to_cycle(unsigned long time) > >> > +static u32 cycle_to_time(u32 cycle) > >> > +static int ftm_set_alarm(u64 cycle) > >> > +static irqreturn_t ftm_alarm_interrupt(int irq, void *dev_id) > >> > +static ssize_t ftm_alarm_show(struct device *dev, > >> > + struct device_attribute *attr, > >> > + char *buf) > >> > +static ssize_t ftm_alarm_store(struct device *dev, > >> > + struct device_attribute *attr, > >> > + const char *buf, size_t count) > >> (...) > >> > +static struct device_attribute ftm_alarm_attributes = __ATTR(ftm_alarm, > 0644, > >> > + ftm_alarm_show, ftm_alarm_store); > >> > >> If you're gonna invent ABIs, document then in Documentation/ABI/testing/*. > >> > >> But I don't get it. Why is this driver not in drivers/rtc? > >> > >> It does a subset of what an RTC does. The ioctl()'s of an RTC > >> can do what you want to do. And much much more. > >> > >> If it can't do all an RTC can do, surely the RTC subsystem > >> can be augmented to host it anyway. It's way to close to > >> an RTC to have it's own random sysfs driver like this. > >> > >> Unless I'm totally off, rewrite this to an RTC driver and post > >> it to the RTC maintainers. > > > > FlexTimer is not a RTC device and not have any rtc deivce function. They > belong to > > different devices, why we need to register this to RTC framework? I am > confused about this. > > > > Now in freescale layerscape platform this driver is only for FlexTimer0, and > not > > fit for each flextimer. Because only FlexTimer0 still turn-on when system in > the Deep Sleep. > > > > If the "alarm" make you feel confused or mislead you think this is a RTC > devices. I think > > I need to change the "alarm" to "timer". > > I think it is an RTC, it is just that the hardware engineer > designed it with a wakeup usecase in mind and did not call > it an RTC. Wakeup is one of the things RTCs do. > > If you inspect a few drivers in drivers/rtc such as drivers/rtc/rtc-pl030.c > you will find that they are just as crude as this "alarm" thing. > > It has a counter that counts cycles, it has a comparator > and an alarm function. It is an on-chip RTC, just like PL030 > no matter what the datasheet or hardware engineer thinks it > should be called, the Linux kernel calls this an RTC, and it > has a subsystem for handling it, so we should use it and > not invent random new stuff. > > If the hardware is really so strange that the counter can only > be started if you also put an alarm at the same time (I doubt > it, but OK if you say so) it is a subset of an RTC that can > only be used for alarms but not timekeeping, but it should > *still* live in drivers/rtc. > > Think for a moment on the huge effort that John Stultz put into > integrating Android alarm timers with POSIX and the RTC > subsystem and fixing it all from the smallest handset to > the largest S360 supercomputer. The approach of a custom > device just throws all of that out the window and reinvents the > mechanism in userspace, forcing all standardized userspace to > have special code to handle this special alarm with its > special sysfs ABI. > > Check > commit ff3ead96d17f47ee70c294a5cc2cce9b61e82f0f > "timers: Introduce in-kernel alarm-timer interface" > for example. > > Even if you persist on keeping it in its own magic driver > like this, it should implement the alarm timer interface > from and I bet after that you don't > need the sysfs files anymore, as the system will sleep > and wake up from the regular syscalls instead of using > magic poking in sysfs from userspace. AFAICT this hardware > is designed for exactly this usecase. > > tools/testing/selftests/timers/alarmtimer-suspend.c > is there for you to test your driver with alarmtimer > support. > > Needless to say: if you implement it as an RTC you get the > alarmtimer interaction for free. That is why we have the > subsystem after all: to be helpful. > > Yours, > Linus Walleij -- -- You received this message because you are subscribed to "rtc-linux". Membership options at http://groups.google.com/group/rtc-linux . Please read http://groups.google.com/group/rtc-linux/web/checklist before submitting a driver. --- You received this message because you are subscribed to the Google Groups "rtc-linux" group. To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com. For more options, visit https://groups.google.com/d/optout. From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dongsheng.Wang@freescale.com (Wang Dongsheng) Date: Fri, 21 Aug 2015 03:07:46 +0000 Subject: [PATCH 2/2] soc/fsl: add ftm alarm driver for ls1021a platform In-Reply-To: References: <1439358807-9024-1-git-send-email-dongsheng.wang@freescale.com> <1439358807-9024-2-git-send-email-dongsheng.wang@freescale.com> Message-ID: To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hi Walleij and Russell, I will drop this patch. Thanks for your review. [PATCH v2 1/2] soc/fsl: add freescale dir for SOC specific drivers But the 1/2 of the patches also need, because there has another patch(Freescale FPGA driver) need 1/2 patch. Need I push the 1/2 patch with another patches(Freescale FPGA driver) or push 1/2 standalone without another patch? Regards, -Dongsheng > -----Original Message----- > From: Linus Walleij [mailto:linus.walleij at linaro.org] > Sent: Friday, August 14, 2015 6:07 PM > To: Wang Dongsheng-B40534; John Stultz > Cc: Alessandro Zummo; Alexandre Belloni; Shawn Guo; Nair, Sandeep; Hans de Goede; > Wang Huan-B18965; linux-arm-kernel at lists.infradead.org; linux- > kernel at vger.kernel.org; rtc-linux at googlegroups.com > Subject: Re: [PATCH 2/2] soc/fsl: add ftm alarm driver for ls1021a platform > > On Fri, Aug 14, 2015 at 5:12 AM, Wang Dongsheng > wrote: > >> On Wed, Aug 12, 2015 at 7:53 AM, Dongsheng Wang > >> wrote: > >> > >> > From: Wang Dongsheng > >> > > >> > Only Ftm0 can be used when system going to deep sleep. So this driver > >> > to support ftm0 as a wakeup source. > >> > > >> > Signed-off-by: Wang Dongsheng > >> > --- > >> > *V2* > >> > Change Copyright 2014 to 2015. > >> (...) > >> > +config FTM_ALARM > >> > + bool "FTM alarm driver" > >> > + depends on SOC_LS1021A > >> > + default n > >> > + help > >> > + Say y here to enable FTM alarm support. The FTM alarm provides > >> > + alarm functions for wakeup system from deep sleep. There is only > >> > + one FTM can be used in ALARM(FTM 0). > >> (...) > >> > +static u32 time_to_cycle(unsigned long time) > >> > +static u32 cycle_to_time(u32 cycle) > >> > +static int ftm_set_alarm(u64 cycle) > >> > +static irqreturn_t ftm_alarm_interrupt(int irq, void *dev_id) > >> > +static ssize_t ftm_alarm_show(struct device *dev, > >> > + struct device_attribute *attr, > >> > + char *buf) > >> > +static ssize_t ftm_alarm_store(struct device *dev, > >> > + struct device_attribute *attr, > >> > + const char *buf, size_t count) > >> (...) > >> > +static struct device_attribute ftm_alarm_attributes = __ATTR(ftm_alarm, > 0644, > >> > + ftm_alarm_show, ftm_alarm_store); > >> > >> If you're gonna invent ABIs, document then in Documentation/ABI/testing/*. > >> > >> But I don't get it. Why is this driver not in drivers/rtc? > >> > >> It does a subset of what an RTC does. The ioctl()'s of an RTC > >> can do what you want to do. And much much more. > >> > >> If it can't do all an RTC can do, surely the RTC subsystem > >> can be augmented to host it anyway. It's way to close to > >> an RTC to have it's own random sysfs driver like this. > >> > >> Unless I'm totally off, rewrite this to an RTC driver and post > >> it to the RTC maintainers. > > > > FlexTimer is not a RTC device and not have any rtc deivce function. They > belong to > > different devices, why we need to register this to RTC framework? I am > confused about this. > > > > Now in freescale layerscape platform this driver is only for FlexTimer0, and > not > > fit for each flextimer. Because only FlexTimer0 still turn-on when system in > the Deep Sleep. > > > > If the "alarm" make you feel confused or mislead you think this is a RTC > devices. I think > > I need to change the "alarm" to "timer". > > I think it is an RTC, it is just that the hardware engineer > designed it with a wakeup usecase in mind and did not call > it an RTC. Wakeup is one of the things RTCs do. > > If you inspect a few drivers in drivers/rtc such as drivers/rtc/rtc-pl030.c > you will find that they are just as crude as this "alarm" thing. > > It has a counter that counts cycles, it has a comparator > and an alarm function. It is an on-chip RTC, just like PL030 > no matter what the datasheet or hardware engineer thinks it > should be called, the Linux kernel calls this an RTC, and it > has a subsystem for handling it, so we should use it and > not invent random new stuff. > > If the hardware is really so strange that the counter can only > be started if you also put an alarm at the same time (I doubt > it, but OK if you say so) it is a subset of an RTC that can > only be used for alarms but not timekeeping, but it should > *still* live in drivers/rtc. > > Think for a moment on the huge effort that John Stultz put into > integrating Android alarm timers with POSIX and the RTC > subsystem and fixing it all from the smallest handset to > the largest S360 supercomputer. The approach of a custom > device just throws all of that out the window and reinvents the > mechanism in userspace, forcing all standardized userspace to > have special code to handle this special alarm with its > special sysfs ABI. > > Check > commit ff3ead96d17f47ee70c294a5cc2cce9b61e82f0f > "timers: Introduce in-kernel alarm-timer interface" > for example. > > Even if you persist on keeping it in its own magic driver > like this, it should implement the alarm timer interface > from and I bet after that you don't > need the sysfs files anymore, as the system will sleep > and wake up from the regular syscalls instead of using > magic poking in sysfs from userspace. AFAICT this hardware > is designed for exactly this usecase. > > tools/testing/selftests/timers/alarmtimer-suspend.c > is there for you to test your driver with alarmtimer > support. > > Needless to say: if you implement it as an RTC you get the > alarmtimer interaction for free. That is why we have the > subsystem after all: to be helpful. > > Yours, > Linus Walleij