[PATCH 1/6] rtc: rtc-s3c: Fix access unit from byte to word on RTCCON
MyungJoo Ham
myungjoo.ham at gmail.com
Wed Oct 27 03:31:35 EDT 2010
On Fri, Oct 8, 2010 at 8:41 AM, Kukjin Kim <kgene.kim at samsung.com> wrote:
> From: Changhwan Youn <chaos.youn at samsung.com>
>
> S3C2410_RTCCON of TYPE_S3C64XX RTC should be read/written by
> readw and writew, because TYPE_S3C64XX RTC uses bit 8 and 9.
> And TYPE_S3C2410 RTC also can access it by readw and writew.
>
> Signed-off-by: Changhwan Youn <chaos.youn at samsung.com>
> [atul.dahiya at samsung.com: tested on smdk2416]
> Tested-by: Atul Dahiya <atul.dahiya at samsung.com>
> Signed-off-by: Kukjin Kim <kgene.kim at samsung.com>
> Cc: Ben Dooks <ben-linux at fluff.org>
> ---
> drivers/rtc/rtc-s3c.c | 36 ++++++++++++++++++------------------
> 1 files changed, 18 insertions(+), 18 deletions(-)
Hello,
Sorry for a late reply...
Anyway, I have a small question in this rtc-s3c.c driver.
Is there any reason to use read/write b/w to access registers of rtc-s3c?
Why don't we use readl/writel when accessing registers in this drivers
and just forget which registers require at least 8 or 16 or 32 bits?
In fact, it appears that readw/writew accesses 16bits, not 32bits in
ARM machines, which may incur problems with TICCNT/CURTICCNT
registers.
>
> diff --git a/drivers/rtc/rtc-s3c.c b/drivers/rtc/rtc-s3c.c
> index f57a87f..4a0b875 100644
> --- a/drivers/rtc/rtc-s3c.c
> +++ b/drivers/rtc/rtc-s3c.c
> @@ -100,7 +100,7 @@ static int s3c_rtc_setpie(struct device *dev, int enabled)
> spin_lock_irq(&s3c_rtc_pie_lock);
>
> if (s3c_rtc_cpu_type == TYPE_S3C64XX) {
> - tmp = readb(s3c_rtc_base + S3C2410_RTCCON);
> + tmp = readw(s3c_rtc_base + S3C2410_RTCCON);
> tmp &= ~S3C64XX_RTCCON_TICEN;
>
> if (enabled)
> @@ -318,7 +318,7 @@ static int s3c_rtc_proc(struct device *dev, struct seq_file *seq)
> unsigned int ticnt;
>
> if (s3c_rtc_cpu_type == TYPE_S3C64XX) {
> - ticnt = readb(s3c_rtc_base + S3C2410_RTCCON);
> + ticnt = readw(s3c_rtc_base + S3C2410_RTCCON);
> ticnt &= S3C64XX_RTCCON_TICEN;
> } else {
> ticnt = readb(s3c_rtc_base + S3C2410_TICNT);
> @@ -391,11 +391,11 @@ static void s3c_rtc_enable(struct platform_device *pdev, int en)
> return;
>
> if (!en) {
> - tmp = readb(base + S3C2410_RTCCON);
> + tmp = readw(base + S3C2410_RTCCON);
> if (s3c_rtc_cpu_type == TYPE_S3C64XX)
> tmp &= ~S3C64XX_RTCCON_TICEN;
> tmp &= ~S3C2410_RTCCON_RTCEN;
> - writeb(tmp, base + S3C2410_RTCCON);
> + writew(tmp, base + S3C2410_RTCCON);
>
> if (s3c_rtc_cpu_type == TYPE_S3C2410) {
> tmp = readb(base + S3C2410_TICNT);
> @@ -405,25 +405,25 @@ static void s3c_rtc_enable(struct platform_device *pdev, int en)
> } else {
> /* re-enable the device, and check it is ok */
>
> - if ((readb(base+S3C2410_RTCCON) & S3C2410_RTCCON_RTCEN) == 0){
> + if ((readw(base+S3C2410_RTCCON) & S3C2410_RTCCON_RTCEN) == 0){
> dev_info(&pdev->dev, "rtc disabled, re-enabling\n");
>
> - tmp = readb(base + S3C2410_RTCCON);
> - writeb(tmp|S3C2410_RTCCON_RTCEN, base+S3C2410_RTCCON);
> + tmp = readw(base + S3C2410_RTCCON);
> + writew(tmp|S3C2410_RTCCON_RTCEN, base+S3C2410_RTCCON);
> }
>
> - if ((readb(base + S3C2410_RTCCON) & S3C2410_RTCCON_CNTSEL)){
> + if ((readw(base + S3C2410_RTCCON) & S3C2410_RTCCON_CNTSEL)){
> dev_info(&pdev->dev, "removing RTCCON_CNTSEL\n");
>
> - tmp = readb(base + S3C2410_RTCCON);
> - writeb(tmp& ~S3C2410_RTCCON_CNTSEL, base+S3C2410_RTCCON);
> + tmp = readw(base + S3C2410_RTCCON);
> + writew(tmp& ~S3C2410_RTCCON_CNTSEL, base+S3C2410_RTCCON);
> }
>
> - if ((readb(base + S3C2410_RTCCON) & S3C2410_RTCCON_CLKRST)){
> + if ((readw(base + S3C2410_RTCCON) & S3C2410_RTCCON_CLKRST)){
> dev_info(&pdev->dev, "removing RTCCON_CLKRST\n");
>
> - tmp = readb(base + S3C2410_RTCCON);
> - writeb(tmp & ~S3C2410_RTCCON_CLKRST, base+S3C2410_RTCCON);
> + tmp = readw(base + S3C2410_RTCCON);
> + writew(tmp & ~S3C2410_RTCCON_CLKRST, base+S3C2410_RTCCON);
> }
> }
> }
> @@ -514,8 +514,8 @@ static int __devinit s3c_rtc_probe(struct platform_device *pdev)
>
> s3c_rtc_enable(pdev, 1);
>
> - pr_debug("s3c2410_rtc: RTCCON=%02x\n",
> - readb(s3c_rtc_base + S3C2410_RTCCON));
> + pr_debug("s3c2410_rtc: RTCCON=%02x\n",
> + readw(s3c_rtc_base + S3C2410_RTCCON));
>
> device_init_wakeup(&pdev->dev, 1);
>
> @@ -578,7 +578,7 @@ static int s3c_rtc_suspend(struct platform_device *pdev, pm_message_t state)
> /* save TICNT for anyone using periodic interrupts */
> ticnt_save = readb(s3c_rtc_base + S3C2410_TICNT);
> if (s3c_rtc_cpu_type == TYPE_S3C64XX) {
> - ticnt_en_save = readb(s3c_rtc_base + S3C2410_RTCCON);
> + ticnt_en_save = readw(s3c_rtc_base + S3C2410_RTCCON);
> ticnt_en_save &= S3C64XX_RTCCON_TICEN;
> }
> s3c_rtc_enable(pdev, 0);
> @@ -596,8 +596,8 @@ static int s3c_rtc_resume(struct platform_device *pdev)
> s3c_rtc_enable(pdev, 1);
> writeb(ticnt_save, s3c_rtc_base + S3C2410_TICNT);
> if (s3c_rtc_cpu_type == TYPE_S3C64XX && ticnt_en_save) {
> - tmp = readb(s3c_rtc_base + S3C2410_RTCCON);
> - writeb(tmp | ticnt_en_save, s3c_rtc_base + S3C2410_RTCCON);
> + tmp = readw(s3c_rtc_base + S3C2410_RTCCON);
> + writew(tmp | ticnt_en_save, s3c_rtc_base + S3C2410_RTCCON);
> }
>
> if (device_may_wakeup(&pdev->dev))
> --
> 1.6.2.5
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
--
MyungJoo Ham, Ph.D.
Mobile Software Platform Lab,
Digital Media and Communications (DMC) Business
Samsung Electronics
cell: 82-10-6714-2858
More information about the linux-arm-kernel
mailing list