* [PATCH net-next] net: enetc: use get/put_unaligned() for mac address handling
@ 2021-06-04 12:30 Michael Walle
2021-06-04 12:44 ` Heiner Kallweit
0 siblings, 1 reply; 4+ messages in thread
From: Michael Walle @ 2021-06-04 12:30 UTC (permalink / raw
To: netdev, linux-kernel
Cc: Claudiu Manoil, David S . Miller, Jakub Kicinski, Vladimir Oltean,
Michael Walle
The supplied buffer for the MAC address might not be aligned. Thus
doing a 32bit (or 16bit) access could be on an unaligned address. For
now, enetc is only used on aarch64 which can do unaligned accesses, thus
there is no error. In any case, be correct and use the get/put_unaligned()
helpers.
Signed-off-by: Michael Walle <michael@walle.cc>
---
drivers/net/ethernet/freescale/enetc/enetc_pf.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/freescale/enetc/enetc_pf.c b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
index 31274325159a..a96d2acb5e11 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_pf.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
@@ -1,6 +1,7 @@
// SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause)
/* Copyright 2017-2019 NXP */
+#include <asm/unaligned.h>
#include <linux/mdio.h>
#include <linux/module.h>
#include <linux/fsl/enetc_mdio.h>
@@ -17,15 +18,15 @@ static void enetc_pf_get_primary_mac_addr(struct enetc_hw *hw, int si, u8 *addr)
u32 upper = __raw_readl(hw->port + ENETC_PSIPMAR0(si));
u16 lower = __raw_readw(hw->port + ENETC_PSIPMAR1(si));
- *(u32 *)addr = upper;
- *(u16 *)(addr + 4) = lower;
+ put_unaligned(upper, (u32 *)addr);
+ put_unaligned(lower, (u16 *)(addr + 4));
}
static void enetc_pf_set_primary_mac_addr(struct enetc_hw *hw, int si,
const u8 *addr)
{
- u32 upper = *(const u32 *)addr;
- u16 lower = *(const u16 *)(addr + 4);
+ u32 upper = get_unaligned((const u32 *)addr);
+ u16 lower = get_unaligned((const u16 *)(addr + 4));
__raw_writel(upper, hw->port + ENETC_PSIPMAR0(si));
__raw_writew(lower, hw->port + ENETC_PSIPMAR1(si));
--
2.20.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH net-next] net: enetc: use get/put_unaligned() for mac address handling
2021-06-04 12:30 [PATCH net-next] net: enetc: use get/put_unaligned() for mac address handling Michael Walle
@ 2021-06-04 12:44 ` Heiner Kallweit
2021-06-04 13:27 ` Michael Walle
2021-06-04 13:34 ` Claudiu Manoil
0 siblings, 2 replies; 4+ messages in thread
From: Heiner Kallweit @ 2021-06-04 12:44 UTC (permalink / raw
To: Michael Walle, netdev, linux-kernel
Cc: Claudiu Manoil, David S . Miller, Jakub Kicinski, Vladimir Oltean
On 04.06.2021 14:30, Michael Walle wrote:
> The supplied buffer for the MAC address might not be aligned. Thus
> doing a 32bit (or 16bit) access could be on an unaligned address. For
> now, enetc is only used on aarch64 which can do unaligned accesses, thus
> there is no error. In any case, be correct and use the get/put_unaligned()
> helpers.
>
> Signed-off-by: Michael Walle <michael@walle.cc>
> ---
> drivers/net/ethernet/freescale/enetc/enetc_pf.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/freescale/enetc/enetc_pf.c b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
> index 31274325159a..a96d2acb5e11 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc_pf.c
> +++ b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
> @@ -1,6 +1,7 @@
> // SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause)
> /* Copyright 2017-2019 NXP */
>
> +#include <asm/unaligned.h>
> #include <linux/mdio.h>
> #include <linux/module.h>
> #include <linux/fsl/enetc_mdio.h>
> @@ -17,15 +18,15 @@ static void enetc_pf_get_primary_mac_addr(struct enetc_hw *hw, int si, u8 *addr)
> u32 upper = __raw_readl(hw->port + ENETC_PSIPMAR0(si));
> u16 lower = __raw_readw(hw->port + ENETC_PSIPMAR1(si));
>
> - *(u32 *)addr = upper;
> - *(u16 *)(addr + 4) = lower;
> + put_unaligned(upper, (u32 *)addr);
> + put_unaligned(lower, (u16 *)(addr + 4));
I think you want to write little endian, therefore on a BE platform
this code may be wrong. Better use put_unaligned_le32?
By using these versions of the unaligned helpers you could also
remove the pointer cast.
> }
>
> static void enetc_pf_set_primary_mac_addr(struct enetc_hw *hw, int si,
> const u8 *addr)
> {
> - u32 upper = *(const u32 *)addr;
> - u16 lower = *(const u16 *)(addr + 4);
> + u32 upper = get_unaligned((const u32 *)addr);
> + u16 lower = get_unaligned((const u16 *)(addr + 4));
>
> __raw_writel(upper, hw->port + ENETC_PSIPMAR0(si));
> __raw_writew(lower, hw->port + ENETC_PSIPMAR1(si));
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH net-next] net: enetc: use get/put_unaligned() for mac address handling
2021-06-04 12:44 ` Heiner Kallweit
@ 2021-06-04 13:27 ` Michael Walle
2021-06-04 13:34 ` Claudiu Manoil
1 sibling, 0 replies; 4+ messages in thread
From: Michael Walle @ 2021-06-04 13:27 UTC (permalink / raw
To: Heiner Kallweit
Cc: netdev, linux-kernel, Claudiu Manoil, David S . Miller,
Jakub Kicinski, Vladimir Oltean
Am 2021-06-04 14:44, schrieb Heiner Kallweit:
> On 04.06.2021 14:30, Michael Walle wrote:
>> The supplied buffer for the MAC address might not be aligned. Thus
>> doing a 32bit (or 16bit) access could be on an unaligned address. For
>> now, enetc is only used on aarch64 which can do unaligned accesses,
>> thus
>> there is no error. In any case, be correct and use the
>> get/put_unaligned()
>> helpers.
>>
>> Signed-off-by: Michael Walle <michael@walle.cc>
>> ---
>> drivers/net/ethernet/freescale/enetc/enetc_pf.c | 9 +++++----
>> 1 file changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/freescale/enetc/enetc_pf.c
>> b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
>> index 31274325159a..a96d2acb5e11 100644
>> --- a/drivers/net/ethernet/freescale/enetc/enetc_pf.c
>> +++ b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
>> @@ -1,6 +1,7 @@
>> // SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause)
>> /* Copyright 2017-2019 NXP */
>>
>> +#include <asm/unaligned.h>
>> #include <linux/mdio.h>
>> #include <linux/module.h>
>> #include <linux/fsl/enetc_mdio.h>
>> @@ -17,15 +18,15 @@ static void enetc_pf_get_primary_mac_addr(struct
>> enetc_hw *hw, int si, u8 *addr)
>> u32 upper = __raw_readl(hw->port + ENETC_PSIPMAR0(si));
>> u16 lower = __raw_readw(hw->port + ENETC_PSIPMAR1(si));
>>
>> - *(u32 *)addr = upper;
>> - *(u16 *)(addr + 4) = lower;
>> + put_unaligned(upper, (u32 *)addr);
>> + put_unaligned(lower, (u16 *)(addr + 4));
>
> I think you want to write little endian, therefore on a BE platform
> this code may be wrong. Better use put_unaligned_le32?
> By using these versions of the unaligned helpers you could also
> remove the pointer cast.
I wasn't sure about the endianness. Might be the case, that on BE
platforms, the endianess of the register will swap too. (OTOH I
could use aarch64 with BE..)
But I'm fine with changing it. I'd presume this being the only
platform for now it doesn't really matter.
-michael
^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [PATCH net-next] net: enetc: use get/put_unaligned() for mac address handling
2021-06-04 12:44 ` Heiner Kallweit
2021-06-04 13:27 ` Michael Walle
@ 2021-06-04 13:34 ` Claudiu Manoil
1 sibling, 0 replies; 4+ messages in thread
From: Claudiu Manoil @ 2021-06-04 13:34 UTC (permalink / raw
To: Heiner Kallweit, Michael Walle, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
Cc: David S . Miller, Jakub Kicinski, Vladimir Oltean
> -----Original Message-----
> From: Heiner Kallweit <hkallweit1@gmail.com>
> Sent: Friday, June 4, 2021 3:44 PM
[...]
>
> On 04.06.2021 14:30, Michael Walle wrote:
> > The supplied buffer for the MAC address might not be aligned. Thus
> > doing a 32bit (or 16bit) access could be on an unaligned address. For
> > now, enetc is only used on aarch64 which can do unaligned accesses, thus
> > there is no error. In any case, be correct and use the get/put_unaligned()
> > helpers.
> >
> > Signed-off-by: Michael Walle <michael@walle.cc>
> > ---
> > drivers/net/ethernet/freescale/enetc/enetc_pf.c | 9 +++++----
> > 1 file changed, 5 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/freescale/enetc/enetc_pf.c
> b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
> > index 31274325159a..a96d2acb5e11 100644
> > --- a/drivers/net/ethernet/freescale/enetc/enetc_pf.c
> > +++ b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
> > @@ -1,6 +1,7 @@
> > // SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause)
> > /* Copyright 2017-2019 NXP */
> >
> > +#include <asm/unaligned.h>
> > #include <linux/mdio.h>
> > #include <linux/module.h>
> > #include <linux/fsl/enetc_mdio.h>
> > @@ -17,15 +18,15 @@ static void enetc_pf_get_primary_mac_addr(struct
> enetc_hw *hw, int si, u8 *addr)
> > u32 upper = __raw_readl(hw->port + ENETC_PSIPMAR0(si));
> > u16 lower = __raw_readw(hw->port + ENETC_PSIPMAR1(si));
> >
> > - *(u32 *)addr = upper;
> > - *(u16 *)(addr + 4) = lower;
> > + put_unaligned(upper, (u32 *)addr);
> > + put_unaligned(lower, (u16 *)(addr + 4));
>
> I think you want to write little endian, therefore on a BE platform
> this code may be wrong. Better use put_unaligned_le32?
> By using these versions of the unaligned helpers you could also
> remove the pointer cast.
>
+1
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-06-04 13:35 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-06-04 12:30 [PATCH net-next] net: enetc: use get/put_unaligned() for mac address handling Michael Walle
2021-06-04 12:44 ` Heiner Kallweit
2021-06-04 13:27 ` Michael Walle
2021-06-04 13:34 ` Claudiu Manoil
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.