From: Stefano Babic <sbabic@denx.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] ATA: Squash warnings in mxc_ata.
Date: Fri, 23 Sep 2011 11:27:36 +0200 [thread overview]
Message-ID: <4E7C5108.2080700@denx.de> (raw)
In-Reply-To: <1316769132-20368-1-git-send-email-marek.vasut@gmail.com>
On 09/23/2011 11:12 AM, Marek Vasut wrote:
Hi Marek,
> mxc_ata.c: In function ?set_ata_bus_timing?:
> mxc_ata.c:118: warning: dereferencing type-punned pointer will break
> strict-aliasing rules
> mxc_ata.c:125: warning: dereferencing type-punned pointer will break
> strict-aliasing rules
> mxc_ata.c:129: warning: dereferencing type-punned pointer will break
> strict-aliasing rules
>
> Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
> Cc: Stefano Babic <sbabic@denx.de>
> ---
> drivers/block/mxc_ata.c | 10 +++++++---
> 1 files changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/block/mxc_ata.c b/drivers/block/mxc_ata.c
> index f22f4f4..d94ec9c 100644
> --- a/drivers/block/mxc_ata.c
> +++ b/drivers/block/mxc_ata.c
> @@ -98,6 +98,7 @@ static void set_ata_bus_timing(unsigned char mode)
> {
> uint32_t val;
> uint32_t T = 1000000000 / mxc_get_clock(MXC_IPG_CLK);
> + uint32_t reg;
>
> struct mxc_ata_config_regs *ata_regs;
> ata_regs = (struct mxc_ata_config_regs *)CONFIG_SYS_ATA_BASE_ADDR;
> @@ -106,22 +107,25 @@ static void set_ata_bus_timing(unsigned char mode)
> return;
>
> /* Write TIME_OFF/ON/1/2W */
> + reg = (uint32_t)&ata_regs->time_off;
You drop the warnings using casting, but is it correct ? time_off is
defined as u8 (correctly, because this is the size of the register). If
you write 32bit, you risk to overwrite the time_on, time_1 and time_2w
registers.
Why do not use writeb ?
> val = (3 << REG2OFF(&ata_regs->time_off)) |
> (3 << REG2OFF(&ata_regs->time_on)) |
> (((pio_t1[mode] + T) / T) << REG2OFF(&ata_regs->time_1)) |
> (((pio_t2_8[mode] + T) / T) << REG2OFF(&ata_regs->time_2w));
> - writel(val, &ata_regs->time_off);
> + writel(val, reg);
Understood. You *want* to write all registers in one shot. However this
is hackish. Or you write each register separetely, or you use a union to
access the register block with a 32 bit access. Everybody reading
"writel(val, &ata_regs->time_off)" expects that time_off is 32bit wide.
Best regards,
Stefano Babic
--
=====================================================================
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office at denx.de
=====================================================================
next prev parent reply other threads:[~2011-09-23 9:27 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-09-23 9:12 [U-Boot] [PATCH] ATA: Squash warnings in mxc_ata Marek Vasut
2011-09-23 9:27 ` Stefano Babic [this message]
2011-09-23 9:42 ` [U-Boot] [PATCH V2] " Marek Vasut
2011-09-23 10:02 ` Stefano Babic
2011-09-23 10:04 ` [U-Boot] [PATCH V3] " Marek Vasut
2011-09-25 17:21 ` Stefano Babic
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4E7C5108.2080700@denx.de \
--to=sbabic@denx.de \
--cc=u-boot@lists.denx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.