Hi Peter,
This patch series moves the existing {get,put}_unaligned_[bl]e24() definitions
into include/linux/unaligned/generic.h. This patch series also introduces a function
for sign-extending 24-bit into 32-bit integers and introduces users for all new
functions and macros. Please consider this patch series for kernel version v5.5.
Thanks,
Bart.
Bart Van Assche (9):
linux/unaligned/byteshift.h: Remove superfluous casts
c6x: Include <linux/unaligned/generic.h> instead of duplicating it
treewide: Consolidate {get,put}_unaligned_[bl]e24() definitions
drivers/iio: Sign extend without triggering implementation-defined
behavior
scsi/st: Use get_unaligned_signed_be24()
scsi/trace: Use get_unaligned_be*()
arm/ecard: Use get_unaligned_le{16,24}()
IB/qib: Sign extend without triggering implementation-defined behavior
ASoC/fsl_spdif: Use put_unaligned_be24() instead of open-coding it
arch/arm/mach-rpc/ecard.c | 18 +--
arch/c6x/include/asm/unaligned.h | 65 +--------
.../iio/common/st_sensors/st_sensors_core.c | 7 +-
drivers/infiniband/hw/qib/qib_rc.c | 2 +-
drivers/nvme/host/rdma.c | 8 --
drivers/nvme/target/rdma.c | 6 -
drivers/scsi/scsi_trace.c | 128 ++++++------------
drivers/scsi/st.c | 4 +-
drivers/usb/gadget/function/f_mass_storage.c | 1 +
drivers/usb/gadget/function/storage_common.h | 5 -
include/linux/unaligned/be_byteshift.h | 6 +-
include/linux/unaligned/generic.h | 44 ++++++
include/linux/unaligned/le_byteshift.h | 6 +-
include/target/target_core_backend.h | 6 -
sound/soc/fsl/fsl_spdif.c | 5 +-
15 files changed, 103 insertions(+), 208 deletions(-)
From the C standard: "The result of E1 >> E2 is E1 right-shifted E2 bit
positions. If E1 has an unsigned type or if E1 has a signed type and a
nonnegative value, the value of the result is the integral part of the
quotient of E1 / 2E2 . If E1 has a signed type and a negative value, the
resulting value is implementation-defined."
Hence use sign_extend_24_to_32() instead of "<< 8 >> 8".
Cc: Dennis Dalessandro <[email protected]>
Cc: Mike Marciniszyn <[email protected]>
Cc: Jason Gunthorpe <[email protected]>
Cc: Doug Ledford <[email protected]>
Signed-off-by: Bart Van Assche <[email protected]>
---
drivers/infiniband/hw/qib/qib_rc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/infiniband/hw/qib/qib_rc.c b/drivers/infiniband/hw/qib/qib_rc.c
index aaf7438258fa..2f1beaab6935 100644
--- a/drivers/infiniband/hw/qib/qib_rc.c
+++ b/drivers/infiniband/hw/qib/qib_rc.c
@@ -566,7 +566,7 @@ int qib_make_rc_req(struct rvt_qp *qp, unsigned long *flags)
break;
}
qp->s_sending_hpsn = bth2;
- delta = (((int) bth2 - (int) wqe->psn) << 8) >> 8;
+ delta = sign_extend_24_to_32(bth2 - wqe->psn);
if (delta && delta % QIB_PSN_CREDIT == 0)
bth2 |= IB_BTH_REQ_ACK;
if (qp->s_flags & RVT_S_SEND_ONE) {
--
2.24.0.rc0.303.g954a862665-goog
Use the generic __{get,put}_unaligned_[bl]e() definitions instead of
duplicating these. Since a later patch will add more definitions into
<linux/unaligned/generic.h>, this patch ensures that these definitions
have to be added only once. See also commit a7f626c1948a ("C6X: headers").
See also commit 6510d41954dc ("kernel: Move arches to use common unaligned
access").
Cc: Mark Salter <[email protected]>
Cc: Aurelien Jacquiot <[email protected]>
Signed-off-by: Bart Van Assche <[email protected]>
---
arch/c6x/include/asm/unaligned.h | 65 +-------------------------------
1 file changed, 1 insertion(+), 64 deletions(-)
diff --git a/arch/c6x/include/asm/unaligned.h b/arch/c6x/include/asm/unaligned.h
index b56ba7110f5a..d628cc170564 100644
--- a/arch/c6x/include/asm/unaligned.h
+++ b/arch/c6x/include/asm/unaligned.h
@@ -10,6 +10,7 @@
#define _ASM_C6X_UNALIGNED_H
#include <linux/swab.h>
+#include <linux/unaligned/generic.h>
/*
* The C64x+ can do unaligned word and dword accesses in hardware
@@ -100,68 +101,4 @@ static inline void put_unaligned64(u64 val, const void *p)
#endif
-/*
- * Cause a link-time error if we try an unaligned access other than
- * 1,2,4 or 8 bytes long
- */
-extern int __bad_unaligned_access_size(void);
-
-#define __get_unaligned_le(ptr) (typeof(*(ptr)))({ \
- sizeof(*(ptr)) == 1 ? *(ptr) : \
- (sizeof(*(ptr)) == 2 ? get_unaligned_le16((ptr)) : \
- (sizeof(*(ptr)) == 4 ? get_unaligned_le32((ptr)) : \
- (sizeof(*(ptr)) == 8 ? get_unaligned_le64((ptr)) : \
- __bad_unaligned_access_size()))); \
- })
-
-#define __get_unaligned_be(ptr) (__force typeof(*(ptr)))({ \
- sizeof(*(ptr)) == 1 ? *(ptr) : \
- (sizeof(*(ptr)) == 2 ? get_unaligned_be16((ptr)) : \
- (sizeof(*(ptr)) == 4 ? get_unaligned_be32((ptr)) : \
- (sizeof(*(ptr)) == 8 ? get_unaligned_be64((ptr)) : \
- __bad_unaligned_access_size()))); \
- })
-
-#define __put_unaligned_le(val, ptr) ({ \
- void *__gu_p = (ptr); \
- switch (sizeof(*(ptr))) { \
- case 1: \
- *(u8 *)__gu_p = (__force u8)(val); \
- break; \
- case 2: \
- put_unaligned_le16((__force u16)(val), __gu_p); \
- break; \
- case 4: \
- put_unaligned_le32((__force u32)(val), __gu_p); \
- break; \
- case 8: \
- put_unaligned_le64((__force u64)(val), __gu_p); \
- break; \
- default: \
- __bad_unaligned_access_size(); \
- break; \
- } \
- (void)0; })
-
-#define __put_unaligned_be(val, ptr) ({ \
- void *__gu_p = (ptr); \
- switch (sizeof(*(ptr))) { \
- case 1: \
- *(u8 *)__gu_p = (__force u8)(val); \
- break; \
- case 2: \
- put_unaligned_be16((__force u16)(val), __gu_p); \
- break; \
- case 4: \
- put_unaligned_be32((__force u32)(val), __gu_p); \
- break; \
- case 8: \
- put_unaligned_be64((__force u64)(val), __gu_p); \
- break; \
- default: \
- __bad_unaligned_access_size(); \
- break; \
- } \
- (void)0; })
-
#endif /* _ASM_C6X_UNALIGNED_H */
--
2.24.0.rc0.303.g954a862665-goog
Move the get_unaligned_be24(), get_unaligned_le24() and
put_unaligned_le24() definitions from various drivers into
include/linux/unaligned/generic.h. Add a put_unaligned_be24() and
get_unaligned_signed_[bl]e24() definitions. Change the functions that
depend on get_unaligned_be32() into macros because
<linux/unaligned/generic.h> may be included before get_unaligned_be32()
has been redefined as a macro.
Cc: Christoph Hellwig <[email protected]>
Cc: Keith Busch <[email protected]>
Cc: Sagi Grimberg <[email protected]>
Cc: Jens Axboe <[email protected]>
Cc: Felipe Balbi <[email protected]>
Cc: Harvey Harrison <[email protected]>
Cc: Martin K. Petersen <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: H. Peter Anvin <[email protected]>
Cc: Andrew Morton <[email protected]>
Signed-off-by: Bart Van Assche <[email protected]>
---
drivers/nvme/host/rdma.c | 8 ----
drivers/nvme/target/rdma.c | 6 ---
drivers/usb/gadget/function/f_mass_storage.c | 1 +
drivers/usb/gadget/function/storage_common.h | 5 ---
include/linux/unaligned/generic.h | 44 ++++++++++++++++++++
include/target/target_core_backend.h | 6 ---
6 files changed, 45 insertions(+), 25 deletions(-)
diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c
index dfa07bb9dfeb..66d9c8cc0c5c 100644
--- a/drivers/nvme/host/rdma.c
+++ b/drivers/nvme/host/rdma.c
@@ -142,14 +142,6 @@ static void nvme_rdma_recv_done(struct ib_cq *cq, struct ib_wc *wc);
static const struct blk_mq_ops nvme_rdma_mq_ops;
static const struct blk_mq_ops nvme_rdma_admin_mq_ops;
-/* XXX: really should move to a generic header sooner or later.. */
-static inline void put_unaligned_le24(u32 val, u8 *p)
-{
- *p++ = val;
- *p++ = val >> 8;
- *p++ = val >> 16;
-}
-
static inline int nvme_rdma_queue_idx(struct nvme_rdma_queue *queue)
{
return queue - queue->ctrl->queues;
diff --git a/drivers/nvme/target/rdma.c b/drivers/nvme/target/rdma.c
index 36d906a7f70d..dc193526d4da 100644
--- a/drivers/nvme/target/rdma.c
+++ b/drivers/nvme/target/rdma.c
@@ -143,12 +143,6 @@ static int num_pages(int len)
return 1 + (((len - 1) & PAGE_MASK) >> PAGE_SHIFT);
}
-/* XXX: really should move to a generic header sooner or later.. */
-static inline u32 get_unaligned_le24(const u8 *p)
-{
- return (u32)p[0] | (u32)p[1] << 8 | (u32)p[2] << 16;
-}
-
static inline bool nvmet_rdma_need_data_in(struct nvmet_rdma_rsp *rsp)
{
return nvme_is_write(rsp->req.cmd) &&
diff --git a/drivers/usb/gadget/function/f_mass_storage.c b/drivers/usb/gadget/function/f_mass_storage.c
index 7c96c4665178..950d2a85f098 100644
--- a/drivers/usb/gadget/function/f_mass_storage.c
+++ b/drivers/usb/gadget/function/f_mass_storage.c
@@ -216,6 +216,7 @@
#include <linux/freezer.h>
#include <linux/module.h>
#include <linux/uaccess.h>
+#include <asm/unaligned.h>
#include <linux/usb/ch9.h>
#include <linux/usb/gadget.h>
diff --git a/drivers/usb/gadget/function/storage_common.h b/drivers/usb/gadget/function/storage_common.h
index e5e3a2553aaa..bdeb1e233fc9 100644
--- a/drivers/usb/gadget/function/storage_common.h
+++ b/drivers/usb/gadget/function/storage_common.h
@@ -172,11 +172,6 @@ enum data_direction {
DATA_DIR_NONE
};
-static inline u32 get_unaligned_be24(u8 *buf)
-{
- return 0xffffff & (u32) get_unaligned_be32(buf - 1);
-}
-
static inline struct fsg_lun *fsg_lun_from_dev(struct device *dev)
{
return container_of(dev, struct fsg_lun, dev);
diff --git a/include/linux/unaligned/generic.h b/include/linux/unaligned/generic.h
index 57d3114656e5..f7fa3f248c85 100644
--- a/include/linux/unaligned/generic.h
+++ b/include/linux/unaligned/generic.h
@@ -2,6 +2,8 @@
#ifndef _LINUX_UNALIGNED_GENERIC_H
#define _LINUX_UNALIGNED_GENERIC_H
+#include <linux/types.h>
+
/*
* Cause a link-time error if we try an unaligned access other than
* 1,2,4 or 8 bytes long
@@ -66,4 +68,46 @@ extern void __bad_unaligned_access_size(void);
} \
(void)0; })
+/* Only use get_unaligned_be24() if reading p - 1 is allowed. */
+#define get_unaligned_be24(p) (get_unaligned_be32((p) - 1) & 0xffffffu)
+
+#define get_unaligned_le24(p) (get_unaligned_le32((p)) & 0xffffffu)
+
+/* Sign-extend a 24-bit into a 32-bit integer. */
+static inline s32 sign_extend_24_to_32(u32 i)
+{
+ i &= 0xffffffu;
+ return i - ((i >> 23) << 24);
+}
+
+#define get_unaligned_signed_be24(p) \
+ sign_extend_24_to_32(get_unaligned_be24((p)))
+
+#define get_unaligned_signed_le24(p) \
+ sign_extend_24_to_32(get_unaligned_le24((p)))
+
+static inline void __put_unaligned_be24(u32 val, u8 *p)
+{
+ *p++ = val >> 16;
+ *p++ = val >> 8;
+ *p++ = val;
+}
+
+static inline void put_unaligned_be24(u32 val, void *p)
+{
+ __put_unaligned_be24(val, p);
+}
+
+static inline void __put_unaligned_le24(u32 val, u8 *p)
+{
+ *p++ = val;
+ *p++ = val >> 8;
+ *p++ = val >> 16;
+}
+
+static inline void put_unaligned_le24(u32 val, void *p)
+{
+ __put_unaligned_le24(val, p);
+}
+
#endif /* _LINUX_UNALIGNED_GENERIC_H */
diff --git a/include/target/target_core_backend.h b/include/target/target_core_backend.h
index 51b6f50eabee..1b752d8ea529 100644
--- a/include/target/target_core_backend.h
+++ b/include/target/target_core_backend.h
@@ -116,10 +116,4 @@ static inline bool target_dev_configured(struct se_device *se_dev)
return !!(se_dev->dev_flags & DF_CONFIGURED);
}
-/* Only use get_unaligned_be24() if reading p - 1 is allowed. */
-static inline uint32_t get_unaligned_be24(const uint8_t *const p)
-{
- return get_unaligned_be32(p - 1) & 0xffffffU;
-}
-
#endif /* TARGET_CORE_BACKEND_H */
--
2.24.0.rc0.303.g954a862665-goog
This patch makes the code easier to read.
Cc: Timur Tabi <[email protected]>
Cc: Nicolin Chen <[email protected]>
Cc: Xiubo Li <[email protected]>
Cc: Fabio Estevam <[email protected]>
Cc: Liam Girdwood <[email protected]>
Cc: Mark Brown <[email protected]>
Cc: Jaroslav Kysela <[email protected]>
Cc: Takashi Iwai <[email protected]>
Signed-off-by: Bart Van Assche <[email protected]>
---
sound/soc/fsl/fsl_spdif.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/sound/soc/fsl/fsl_spdif.c b/sound/soc/fsl/fsl_spdif.c
index 7858a5499ac5..8e80ae16f566 100644
--- a/sound/soc/fsl/fsl_spdif.c
+++ b/sound/soc/fsl/fsl_spdif.c
@@ -16,6 +16,7 @@
#include <linux/of_device.h>
#include <linux/of_irq.h>
#include <linux/regmap.h>
+#include <asm/unaligned.h>
#include <sound/asoundef.h>
#include <sound/dmaengine_pcm.h>
@@ -173,9 +174,7 @@ static void spdif_irq_uqrx_full(struct fsl_spdif_priv *spdif_priv, char name)
}
regmap_read(regmap, reg, &val);
- ctrl->subcode[*pos++] = val >> 16;
- ctrl->subcode[*pos++] = val >> 8;
- ctrl->subcode[*pos++] = val;
+ put_unaligned_be24(val, &ctrl->subcode[*pos]);
}
/* U/Q Channel sync found */
--
2.24.0.rc0.303.g954a862665-goog
On 2019-10-28 4:06 p.m., Bart Van Assche wrote:
> Hi Peter,
>
> This patch series moves the existing {get,put}_unaligned_[bl]e24() definitions
> into include/linux/unaligned/generic.h. This patch series also introduces a function
> for sign-extending 24-bit into 32-bit integers and introduces users for all new
> functions and macros. Please consider this patch series for kernel version v5.5.
And while you are at it, the sg3_utils user space copy of
include/linux/unaligned/*.h (called sg_unaligned.h) has bindings
for 48 bit operations.
Checking my sg3_utils code, in VPD page 0x83 (mandatory device
identification page) the EUI-64 based 16-byte designator has a
6 byte field (the "vendor specific extension identifier").
Also the SET TIMESTAMP and REPORT TIMESTAMP commands have a 6 byte
timestamp field. There are also some attribute pages associated with
the READ ATTRIBUTE command that have 6 byte fields.
A recent trend with the pages returned by the SCSI LOG SENSE command
is to have (big endian) fields whose length (in bytes) is encoded
in the response. I have this function for those:
/* Returns 0 if 'num_bytes' is less than or equal to 0 or greater than
* 8 (i.e. sizeof(uint64_t)). Else returns result in uint64_t which is
* an 8 byte unsigned integer. */
static inline uint64_t sg_get_unaligned_be(int num_bytes, const void *p)
And I can see NVMe code in smartmontools using that one:
nvmeprint.cpp: jrns["eui64"]["ext_id"] =
sg_get_unaligned_be(5, id_ns.eui64 + 3);
Doug Gilbert
> Thanks,
>
> Bart.
>
> Bart Van Assche (9):
> linux/unaligned/byteshift.h: Remove superfluous casts
> c6x: Include <linux/unaligned/generic.h> instead of duplicating it
> treewide: Consolidate {get,put}_unaligned_[bl]e24() definitions
> drivers/iio: Sign extend without triggering implementation-defined
> behavior
> scsi/st: Use get_unaligned_signed_be24()
> scsi/trace: Use get_unaligned_be*()
> arm/ecard: Use get_unaligned_le{16,24}()
> IB/qib: Sign extend without triggering implementation-defined behavior
> ASoC/fsl_spdif: Use put_unaligned_be24() instead of open-coding it
>
> arch/arm/mach-rpc/ecard.c | 18 +--
> arch/c6x/include/asm/unaligned.h | 65 +--------
> .../iio/common/st_sensors/st_sensors_core.c | 7 +-
> drivers/infiniband/hw/qib/qib_rc.c | 2 +-
> drivers/nvme/host/rdma.c | 8 --
> drivers/nvme/target/rdma.c | 6 -
> drivers/scsi/scsi_trace.c | 128 ++++++------------
> drivers/scsi/st.c | 4 +-
> drivers/usb/gadget/function/f_mass_storage.c | 1 +
> drivers/usb/gadget/function/storage_common.h | 5 -
> include/linux/unaligned/be_byteshift.h | 6 +-
> include/linux/unaligned/generic.h | 44 ++++++
> include/linux/unaligned/le_byteshift.h | 6 +-
> include/target/target_core_backend.h | 6 -
> sound/soc/fsl/fsl_spdif.c | 5 +-
> 15 files changed, 103 insertions(+), 208 deletions(-)
>
On 2019-10-28 13:06, Bart Van Assche wrote:
> Use the generic __{get,put}_unaligned_[bl]e() definitions instead of
> duplicating these. Since a later patch will add more definitions into
> <linux/unaligned/generic.h>, this patch ensures that these definitions
> have to be added only once. See also commit a7f626c1948a ("C6X: headers").
> See also commit 6510d41954dc ("kernel: Move arches to use common unaligned
> access").
Mark and Aurelien, are you the c6x maintainers? If so, please let me
know whether you agree with this patch.
Thanks,
Bart.
On Mon, 2019-10-28 at 13:06 -0700, Bart Van Assche wrote:
> Use the generic __{get,put}_unaligned_[bl]e() definitions instead of
> duplicating these. Since a later patch will add more definitions into
> <linux/unaligned/generic.h>, this patch ensures that these definitions
> have to be added only once. See also commit a7f626c1948a ("C6X: headers").
> See also commit 6510d41954dc ("kernel: Move arches to use common unaligned
> access").
>
> Cc: Mark Salter <[email protected]>
> Cc: Aurelien Jacquiot <[email protected]>
> Signed-off-by: Bart Van Assche <[email protected]>
> ---
> arch/c6x/include/asm/unaligned.h | 65 +-------------------------------
> 1 file changed, 1 insertion(+), 64 deletions(-)
>
> diff --git a/arch/c6x/include/asm/unaligned.h b/arch/c6x/include/asm/unaligned.h
> index b56ba7110f5a..d628cc170564 100644
> --- a/arch/c6x/include/asm/unaligned.h
> +++ b/arch/c6x/include/asm/unaligned.h
> @@ -10,6 +10,7 @@
> #define _ASM_C6X_UNALIGNED_H
>
> #include <linux/swab.h>
> +#include <linux/unaligned/generic.h>
>
> /*
> * The C64x+ can do unaligned word and dword accesses in hardware
> @@ -100,68 +101,4 @@ static inline void put_unaligned64(u64 val, const void *p)
>
> #endif
>
> -/*
> - * Cause a link-time error if we try an unaligned access other than
> - * 1,2,4 or 8 bytes long
> - */
> -extern int __bad_unaligned_access_size(void);
> -
> -#define __get_unaligned_le(ptr) (typeof(*(ptr)))({ \
> - sizeof(*(ptr)) == 1 ? *(ptr) : \
> - (sizeof(*(ptr)) == 2 ? get_unaligned_le16((ptr)) : \
> - (sizeof(*(ptr)) == 4 ? get_unaligned_le32((ptr)) : \
> - (sizeof(*(ptr)) == 8 ? get_unaligned_le64((ptr)) : \
> - __bad_unaligned_access_size()))); \
> - })
> -
> -#define __get_unaligned_be(ptr) (__force typeof(*(ptr)))({ \
> - sizeof(*(ptr)) == 1 ? *(ptr) : \
> - (sizeof(*(ptr)) == 2 ? get_unaligned_be16((ptr)) : \
> - (sizeof(*(ptr)) == 4 ? get_unaligned_be32((ptr)) : \
> - (sizeof(*(ptr)) == 8 ? get_unaligned_be64((ptr)) : \
> - __bad_unaligned_access_size()))); \
> - })
> -
> -#define __put_unaligned_le(val, ptr) ({ \
> - void *__gu_p = (ptr); \
> - switch (sizeof(*(ptr))) { \
> - case 1: \
> - *(u8 *)__gu_p = (__force u8)(val); \
> - break; \
> - case 2: \
> - put_unaligned_le16((__force u16)(val), __gu_p); \
> - break; \
> - case 4: \
> - put_unaligned_le32((__force u32)(val), __gu_p); \
> - break; \
> - case 8: \
> - put_unaligned_le64((__force u64)(val), __gu_p); \
> - break; \
> - default: \
> - __bad_unaligned_access_size(); \
> - break; \
> - } \
> - (void)0; })
> -
> -#define __put_unaligned_be(val, ptr) ({ \
> - void *__gu_p = (ptr); \
> - switch (sizeof(*(ptr))) { \
> - case 1: \
> - *(u8 *)__gu_p = (__force u8)(val); \
> - break; \
> - case 2: \
> - put_unaligned_be16((__force u16)(val), __gu_p); \
> - break; \
> - case 4: \
> - put_unaligned_be32((__force u32)(val), __gu_p); \
> - break; \
> - case 8: \
> - put_unaligned_be64((__force u64)(val), __gu_p); \
> - break; \
> - default: \
> - __bad_unaligned_access_size(); \
> - break; \
> - } \
> - (void)0; })
> -
> #endif /* _ASM_C6X_UNALIGNED_H */
Acked-by: Mark Salter <[email protected]>