Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Reply-To: dgilbert@interlog.com
Subject: Re: [PATCH 0/9] Consolidate {get,put}_unaligned_[bl]e24() definitions
To:     Bart Van Assche <bvanassche@acm.org>,
        Peter Zijlstra <peterz@infradead.org>
Cc:     Ingo Molnar <mingo@kernel.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        Christoph Hellwig <hch@lst.de>,
        "Martin K . Petersen" <martin.petersen@oracle.com>,
        linux-kernel@vger.kernel.org, linux-scsi@vger.kernel.org
References: <20191028200700.213753-1-bvanassche@acm.org>
From:   Douglas Gilbert <dgilbert@interlog.com>
Message-ID: <21a9443a-b52c-d028-d0a5-2bbc0b4c281a@interlog.com>
Date:   Mon, 28 Oct 2019 21:10:44 -0400
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.9.0
MIME-Version: 1.0
In-Reply-To: <20191028200700.213753-1-bvanassche@acm.org>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-CA
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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(-)
>