Re: [EXT] Re: [PATCH v2] uwb: nxp: sr1xx: UWB driver support for sr1xx series chip

[Manjunatha Venkatesh]

On 3/15/22 17:04, Greg KH wrote:
> Caution: EXT Email
>
> 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.
Sure, will update the description as part of next patch.
>
>> 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@xxxxxxx>
>>
>> Changes since v1:
>> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flkml.org%2Flkml%2F2022%2F3%2F7%2F1254&amp;data=04%7C01%7Cmanjunatha.venkatesh%40nxp.com%7C708a77e63b7d4a5aeca308da0677c19e%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637829408669477610%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=54vIhj3naaT4MRXvX7nUXbaTgx%2BiLoG3NAkh1KqK8V4%3D&amp;reserved=0
>> ---
>>   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@xxxxxxxxxxxxxxxxxxx>
> Hah, NO!
>
>> +M:   manjunatha.venkatesh@xxxxxxx
>> +L:   rvmanjumce@xxxxxxxxx
> 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?
Sorry, Next patch will remove your name from this list.
>
> {sigh}
>
> greg k-h