linux-kernel - Re: [PATCH v2] uwb: nxp: sr1xx: UWB driver support for sr1xx series chip

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 15 Mar 2022 12:34:17 +0100
From:   Greg KH <gregkh@...uxfoundation.org>
To:     Manjunatha Venkatesh <manjunatha.venkatesh@....com>
Cc:     linux-kernel@...r.kernel.org, will@...nel.org, axboe@...nel.dk,
        mb@...htnvm.io, ckeepax@...nsource.cirrus.com, arnd@...db.d,
        mst@...hat.com, javier@...igon.com, mikelley@...rosoft.com,
        jasowang@...hat.com, sunilmut@...rosoft.com,
        bjorn.andersson@...aro.org, rvmanjumce@...il.com
Subject: Re: [PATCH v2] uwb: nxp: sr1xx: UWB driver support for sr1xx series
 chip

On Tue, Mar 15, 2022 at 04:22:05PM +0530, Manjunatha Venkatesh wrote:
> Ultra Wide Band(UWB) is a fast, secure and low power radio protocol used
> to determine location with accuracy unmatched by any other wireless
> technology.

The kernel changelog is not the place for marketing copy.  Please just
describe the hardware without using words like this.

> Its a short-range wireless communication protocol. It uses radio waves to
> enable devices to talk to each other.
> 
> This is a new driver that supports the integrated UWB for
> Nxp SoCs, especially the sr1xx series and depends on the SPI module.
> 
> This driver works with Nxp UWB Subsystem(UWBS) which is FiRa Compliance.
> Corresponding UCI details available in Fira Consortuim website.
> 
> sr1xx is flash less UWBS  and it requires firmware download on every
> device boot.
> 
> Internally driver will handle two modes of operation.
> 1.HBCI mode (sr1xx BootROM Code Interface)
>   Firmware download uses HBCI ptotocol packet structure which is
>   Nxp proprietary,Firmware File(.bin) stored in user space context
>   and during device init sequence pick the firmware packet in chunk
>   and send it to the driver with write() api call.
>   Complete firmware download protocol logic implemented in user space,
>   Driver used here is kind of pipe to send data to SPI line.
>   Firmware acknowledge for every chunk packet sent and same thing
>   is monitored,in user space code(HAL layer).
>   If any error Firmware download sequence will fail and reset the device.
>   If firmware download packet sent successfully at the end device will
>   send device status notification and its indication of device entered
>   UCI mode.Here after any command/response/notification will follow
>   UCI packet structure.
> 
> 2.UCI mode (UWB Command interface)
>   Once Firmware download finishes sr1xx will switch to UCI mode packet
>   structure.Here this driver acts as pipe between user space and sr1xx.
>   Any response or notification received from sr1xx through SPI line
>   will convey to user space.User space(UCI lib) will take care of
>   UCI parsing logic.

Where is that userspace code?  How will it talk to the kernel?  Why do
the existing user/kernel apis not sufice for this one random hardware
device?  We have support for wireless devices of all types today, why
is this so special it needs a custom interface for it?

> 
>   Its IRQ based driver and sr1xx specific irq handshake mechanism logic
>   implemented to avoid any race condition between write and read
>   during ranging sequence.
> 
>   UCI mode Write is same as HBCI mode sequence whatever command received
>   from user space will send to the sr1xx via SPI line.
>   In UCI mode read api called first and waiting on the IRQ line status
>   in order to avoid missing of interrupts after write sequence.
> 
>   This driver needs dts config update as per the sr1xx data sheet.
>   Corresponding document added in Documentation/devicetree/bindings/uwb


> 
> Signed-off-by: Manjunatha Venkatesh <manjunatha.venkatesh@....com>
> 
> Changes since v1:
> https://lkml.org/lkml/2022/3/7/1254
> ---
>  MAINTAINERS          |   7 +
>  drivers/Kconfig      |   2 +
>  drivers/Makefile     |   1 +
>  drivers/uwb/Kconfig  |  27 ++
>  drivers/uwb/Makefile |   5 +
>  drivers/uwb/sr1xx.c  | 857 +++++++++++++++++++++++++++++++++++++++++++
>  6 files changed, 899 insertions(+)
>  create mode 100644 drivers/uwb/Kconfig
>  create mode 100644 drivers/uwb/Makefile
>  create mode 100644 drivers/uwb/sr1xx.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index e127c2fb08a7..31c08ecc152c 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -21471,3 +21471,10 @@ S:	Buried alive in reporters
>  T:	git git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
>  F:	*
>  F:	*/
> +
> +UWB
> +M:	Greg KH <gregkh@...uxfoundation.org>

Hah, NO!

> +M:	manjunatha.venkatesh@....com
> +L:	rvmanjumce@...il.com

Full names.

> +S:	Maintained
> +F:	drivers/uwb/sr1xx.c

This needs to be sorted properly.  Please read the file you are
modifying.

I'm stopping here.  Don't add anyone's name to a MAINTAINERS file unless
you get explicit permission from them to do so.  Otherwise it is quite
rude, don't you think?

{sigh}

greg k-h