* [westeri-thunderbolt:fixes] BUILD SUCCESS ec4405ed92036f5bb487b5c2f9a28f9e36a3e3d5
From: kernel test robot @ 2023-10-05 21:54 UTC (permalink / raw)
To: Mika Westerberg; +Cc: linux-usb
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/westeri/thunderbolt.git fixes
branch HEAD: ec4405ed92036f5bb487b5c2f9a28f9e36a3e3d5 thunderbolt: Call tb_switch_put() once DisplayPort bandwidth request is finished
elapsed time: 731m
configs tested: 133
configs skipped: 2
The following configs have been built successfully.
More configs may be tested in the coming days.
tested configs:
alpha allnoconfig gcc
alpha allyesconfig gcc
alpha defconfig gcc
arc allmodconfig gcc
arc allnoconfig gcc
arc allyesconfig gcc
arc axs101_defconfig gcc
arc defconfig gcc
arc nsimosci_hs_smp_defconfig gcc
arc randconfig-001-20231005 gcc
arm allmodconfig gcc
arm allnoconfig gcc
arm allyesconfig gcc
arm bcm2835_defconfig clang
arm defconfig gcc
arm orion5x_defconfig clang
arm pxa168_defconfig clang
arm randconfig-001-20231005 gcc
arm randconfig-001-20231006 gcc
arm vf610m4_defconfig gcc
arm64 allmodconfig gcc
arm64 allnoconfig gcc
arm64 allyesconfig gcc
arm64 defconfig gcc
csky allmodconfig gcc
csky allnoconfig gcc
csky allyesconfig gcc
csky defconfig gcc
i386 allmodconfig gcc
i386 allnoconfig gcc
i386 allyesconfig gcc
i386 buildonly-randconfig-001-20231005 gcc
i386 buildonly-randconfig-001-20231006 gcc
i386 buildonly-randconfig-002-20231005 gcc
i386 buildonly-randconfig-002-20231006 gcc
i386 buildonly-randconfig-003-20231005 gcc
i386 buildonly-randconfig-003-20231006 gcc
i386 buildonly-randconfig-004-20231005 gcc
i386 buildonly-randconfig-004-20231006 gcc
i386 buildonly-randconfig-005-20231005 gcc
i386 buildonly-randconfig-005-20231006 gcc
i386 buildonly-randconfig-006-20231005 gcc
i386 buildonly-randconfig-006-20231006 gcc
i386 debian-10.3 gcc
i386 defconfig gcc
i386 randconfig-001-20231005 gcc
i386 randconfig-001-20231006 gcc
i386 randconfig-002-20231005 gcc
i386 randconfig-002-20231006 gcc
i386 randconfig-003-20231005 gcc
i386 randconfig-003-20231006 gcc
i386 randconfig-004-20231005 gcc
i386 randconfig-004-20231006 gcc
i386 randconfig-005-20231005 gcc
i386 randconfig-005-20231006 gcc
i386 randconfig-006-20231005 gcc
i386 randconfig-006-20231006 gcc
loongarch allmodconfig gcc
loongarch allnoconfig gcc
loongarch allyesconfig gcc
loongarch defconfig gcc
loongarch randconfig-001-20231005 gcc
m68k allmodconfig gcc
m68k allnoconfig gcc
m68k allyesconfig gcc
m68k defconfig gcc
microblaze allmodconfig gcc
microblaze allnoconfig gcc
microblaze allyesconfig gcc
microblaze defconfig gcc
mips allmodconfig gcc
mips allnoconfig gcc
mips allyesconfig gcc
nios2 allmodconfig gcc
nios2 allnoconfig gcc
nios2 allyesconfig gcc
nios2 defconfig gcc
openrisc allmodconfig gcc
openrisc allnoconfig gcc
openrisc allyesconfig gcc
openrisc defconfig gcc
parisc allmodconfig gcc
parisc allnoconfig gcc
parisc allyesconfig gcc
parisc defconfig gcc
parisc64 defconfig gcc
powerpc allmodconfig gcc
powerpc allnoconfig gcc
powerpc allyesconfig gcc
powerpc bamboo_defconfig gcc
powerpc ep88xc_defconfig gcc
riscv allmodconfig gcc
riscv allnoconfig clang
riscv allnoconfig gcc
riscv allyesconfig gcc
riscv defconfig gcc
riscv rv32_defconfig gcc
s390 alldefconfig clang
s390 allmodconfig gcc
s390 allnoconfig gcc
s390 allyesconfig gcc
s390 defconfig gcc
sh allmodconfig gcc
sh allnoconfig gcc
sh allyesconfig gcc
sh defconfig gcc
sh sdk7780_defconfig gcc
sparc allmodconfig gcc
sparc allnoconfig gcc
sparc allyesconfig gcc
sparc defconfig gcc
sparc sparc32_defconfig gcc
sparc64 allmodconfig gcc
sparc64 allyesconfig gcc
sparc64 defconfig gcc
um allmodconfig clang
um allnoconfig clang
um allyesconfig clang
um defconfig gcc
um i386_defconfig gcc
um x86_64_defconfig gcc
x86_64 allnoconfig gcc
x86_64 allyesconfig gcc
x86_64 defconfig gcc
x86_64 randconfig-001-20231005 gcc
x86_64 randconfig-002-20231005 gcc
x86_64 randconfig-003-20231005 gcc
x86_64 randconfig-004-20231005 gcc
x86_64 randconfig-005-20231005 gcc
x86_64 randconfig-006-20231005 gcc
x86_64 rhel-8.3-rust clang
x86_64 rhel-8.3 gcc
xtensa allnoconfig gcc
xtensa allyesconfig gcc
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH v1 0/3] usb: gadget: uvc: stability fixes on STREAMOFF.
From: Michael Grzeschik @ 2023-10-05 22:05 UTC (permalink / raw)
To: Avichal Rakesh
Cc: Laurent Pinchart, Daniel Scally, Greg Kroah-Hartman, jchowdhary,
etalvala, linux-usb, linux-kernel
In-Reply-To: <53300d24-b558-428d-b52f-316b2e456313@google.com>
[-- Attachment #1: Type: text/plain, Size: 5822 bytes --]
Hi Avichal,
On Thu, Oct 05, 2023 at 11:30:32AM -0700, Avichal Rakesh wrote:
>On 10/5/23 03:14, Michael Grzeschik wrote:
>> On Thu, Oct 05, 2023 at 11:23:27AM +0300, Laurent Pinchart wrote:
>>> On Tue, Oct 03, 2023 at 01:09:06PM +0200, Michael Grzeschik wrote:
>>>> On Sat, Sep 30, 2023 at 11:48:18AM -0700, Avichal Rakesh wrote:
>>>> > We have been seeing two main stability issues that uvc gadget driver
>>>> > runs into when stopping streams:
>>>> > 1. Attempting to queue usb_requests to a disabled usb_ep
>>>> > 2. use-after-free issue for inflight usb_requests
>>>> >
>>>> > The three patches below fix the two issues above. Patch 1/3 fixes the
>>>> > first issue, and Patch 2/3 and 3/3 fix the second issue.
>>>> >
>>>> > Avichal Rakesh (3):
>>>> > usb: gadget: uvc: prevent use of disabled endpoint
>>>> > usb: gadget: uvc: Allocate uvc_requests one at a time
>>>> > usb: gadget: uvc: Fix use-after-free for inflight usb_requests
>>>> >
>>>> > drivers/usb/gadget/function/f_uvc.c | 11 +-
>>>> > drivers/usb/gadget/function/f_uvc.h | 2 +-
>>>> > drivers/usb/gadget/function/uvc.h | 6 +-
>>>> > drivers/usb/gadget/function/uvc_v4l2.c | 21 ++-
>>>> > drivers/usb/gadget/function/uvc_video.c | 189 +++++++++++++++++-------
>>>> > 5 files changed, 164 insertions(+), 65 deletions(-)
>>>>
>>>> These patches are not applying on gregkh/usb-testing since
>>>> Greg did take my patches first. I have already rebased them.
>>>
>>> I think they got merged too soon :-( We could fix things on top, but
>>> there's very little time to do so for v6.7.
>>
>> Agreed. I was jumping from one workaround to another one, since this
>> is not easy to fix in a proper way. And still after this long discussion
>> with Avichal I don't think we are there yet.
>>
>>
>> So far the first two patches from Avichal look legit. But the overall
>> Use-After-Free fix is yet to be done properly.
>>
>> The "abondoned" method he suggested is really bad to follow and will
>> add too much complexity and will be hard to debug.
>>
>> IMHO it should be possible to introduce two cleanup pathes.
>>
>> One path would be in the uvc_cleanup_requests that will cleanup the
>> requests that are actually not used in the controller and are registered
>> in the req_free list.
>>
>> The second path would be the complete functions that are being run
>> from the controller and will ensure that the cleanup will really free
>> the requests from the controller after they were consumed.
>>
>> What do you think?
>
>I am not sure I follow. Patch 3/3 does exactly what you say here.
Yes, it was just to summ up what the latest state of the idea was,
so Laurent does not read the whole thread in detail. Sorry for not
being clear enough about that.
>There are two cleanup paths:
> 1. uvcg_video_disable cleans up only the requests in req_free, and
> 2. complete handler cleans up the in-flight requests.
>
>The "abandoned" flag is simply to let the completion handler know
>which requests to clean up and which ones to re-queue back to
>the gadget driver.
What I don't get is, why in the case of shutdown there needs to
be something re-queued back to the gadget driver. There should not
need to be any sort of barrier flag for the requests. Just the
complete handler running past a barrier where it knows that the
whole device is stopped. So every call on complete should then clean
that exact request it is touching currently.
I don't know where the extra complexity comes from.
>The other "complications" are around making sure we can trust
>the values in an inherently racey situation. The reasoning
>can admittedly be difficult to follow at a glance, which incidentally
>is why I went with a simple to prove timed wait in the past
>(https://lore.kernel.org/20230912041910.726442-3-arakesh@google.com).
>
>I am not suggesting we go back to a timed wait, but please do look
>at the patch and let me know which parts don't make sense, or are
>difficult to understand. We can add more documentation about our
>assumptions there, or if you have a way to do this that you
>think is simpler to reason about, then please let me know and I'll
>be more than happy to use that!
I really try to spin my head around the idea of the is_abondoned flag
you are using. Unfortunatly for now I am out to debug the issues I see
with your series.
So I did try these patches you send. Yes the deadlock error is gone with
v3. But the linked list is still running into cases where
dwc3_gadget_giveback(complete) is touching requests that are already
freed.
[ 61.408715] ------------[ cut here ]------------
[ 61.413897] kernel BUG at lib/list_debug.c:56!
...
[ 61.590762] Call trace:
[ 61.596890] __list_del_entry_valid+0xb8/0xe8
[ 61.603408] dwc3_gadget_giveback+0x3c/0x1b0
[ 61.607594] dwc3_remove_requests.part.0+0xcc/0x100
[ 61.612948] __dwc3_gadget_ep_disable+0xbc/0x1b8
[ 61.621019] dwc3_gadget_ep_disable+0x48/0x100
[ 61.627925] usb_ep_disable+0x3c/0x138
[ 61.638230] uvc_function_setup_continue+0x3c/0x60
[ 61.645040] uvc_v4l2_streamoff+0x5c/0x80
[ 61.659812] v4l_streamoff+0x40/0x60
[ 61.668950] __video_do_ioctl+0x344/0x420
[ 61.679548] video_usercopy+0x1d0/0x788
[ 61.685677] video_ioctl2+0x40/0x70
[ 61.697439] v4l2_ioctl+0x68/0xa0
[ 61.709200] __arm64_sys_ioctl+0x304/0xda0
[ 61.720768] invoke_syscall.constprop.0+0x70/0x130
Regards,
Michael
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: device present in lsusb, disappears in lsusb -t
From: Douglas Gilbert @ 2023-10-06 2:49 UTC (permalink / raw)
To: Greg KH; +Cc: linux-usb@vger.kernel.org
In-Reply-To: <2023091638-duration-barcode-73a3@gregkh>
On 2023-09-16 07:16, Greg KH wrote:
> On Fri, Sep 15, 2023 at 08:16:18PM -0400, Douglas Gilbert wrote:
>> The device in question is this one:
>> Bus 005 Device 015: ID 0483:572b STMicroelectronics STEVAL-USBC2DP Type-C
>> to DisplayPort adapter. It is a USB-C alternate mode device (so tbtadm does
>> not
>> report it).
>>
>> That adapter is connected to a screen (and working) and to a USB-C port on
>> a Lenovo TB3 dock [40AN] which in turn is connected to a Thinkpad X13 Gen3's
>> USB-C port. The Thinkpad is running lk 6.6.0-rc1 with "lsusb (usbutils) 014".
>>
>> The strange thing is that this device is nowhere to be found in the output
>> of "lsusb -t". The lsusb manpage describes the '-t' option as: "Tells
>> lsusb to dump the physical USB device hierarchy as a tree." So is 'physical'
>> a weasel word in this context, or is there a bug in the '-t' option, or is
>> there some other explanation?
>
> A number of 'lsusb -t' issues were fixed in the 015 release of usbutils,
> so maybe update?
>
> that being said, the -t option is a totally different codepath in the
> tool, and shows different things overall. -t shows the drivers that are
> bound to the different interfaces, which means that a single device will
> show up multiple times in the -t option.
>
> Here's the output of the two things on my local laptop, with just a few
> USB devices in it:
>
> $ lsusb
> Bus 004 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub
> Bus 003 Device 003: ID 27c6:609c Shenzhen Goodix Technology Co.,Ltd. Goodix USB2.0 MISC
> Bus 003 Device 006: ID 0bda:5634 Realtek Semiconductor Corp. Laptop Camera
> Bus 003 Device 004: ID 8087:0032 Intel Corp. AX210 Bluetooth
> Bus 003 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
> Bus 002 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub
> Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
>
> $ lsusb -t
> /: Bus 04.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/4p, 10000M
> /: Bus 03.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/12p, 480M
> |__ Port 7: Dev 6, If 0, Class=Video, Driver=uvcvideo, 480M
> |__ Port 7: Dev 6, If 1, Class=Video, Driver=uvcvideo, 480M
> |__ Port 9: Dev 3, If 0, Class=Vendor Specific Class, Driver=, 12M
> |__ Port 10: Dev 4, If 0, Class=Wireless, Driver=btusb, 12M
> |__ Port 10: Dev 4, If 1, Class=Wireless, Driver=btusb, 12M
> /: Bus 02.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/4p, 20000M/x2
> /: Bus 01.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/1p, 480M
>
> and then if you give the -v option as well you see a bit more:
>
> $ lsusb -tv
> /: Bus 04.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/4p, 10000M
> ID 1d6b:0003 Linux Foundation 3.0 root hub
> /: Bus 03.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/12p, 480M
> ID 1d6b:0002 Linux Foundation 2.0 root hub
> |__ Port 7: Dev 6, If 0, Class=Video, Driver=uvcvideo, 480M
> ID 0bda:5634 Realtek Semiconductor Corp.
> |__ Port 7: Dev 6, If 1, Class=Video, Driver=uvcvideo, 480M
> ID 0bda:5634 Realtek Semiconductor Corp.
> |__ Port 9: Dev 3, If 0, Class=Vendor Specific Class, Driver=, 12M
> ID 27c6:609c Shenzhen Goodix Technology Co.,Ltd.
> |__ Port 10: Dev 4, If 0, Class=Wireless, Driver=btusb, 12M
> ID 8087:0032 Intel Corp. AX210 Bluetooth
> |__ Port 10: Dev 4, If 1, Class=Wireless, Driver=btusb, 12M
> ID 8087:0032 Intel Corp. AX210 Bluetooth
> /: Bus 02.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/4p, 20000M/x2
> ID 1d6b:0003 Linux Foundation 3.0 root hub
> /: Bus 01.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/1p, 480M
> ID 1d6b:0002 Linux Foundation 2.0 root hub
>
>
> What are you seeing missing in your output?
lsusb-t.c says:
Copyright (c) 2009 Greg Kroah-Hartman <gregkh@suse.de>
When I tried to contact the author by email off-list with a fair
amount of data (e.g. a copy of /sys from my machine), he claimed
that such an approach was "rude" in the sense that it should have
been sent to this list. Personally I prefer to fix bugs via a
direct email exchange, without the peanut gallery. After all, many
of the bugs found fall into the "brown paper bag" variety.
Plus I felt a bit uncomfortable about publishing the full
contents of /sys from my laptop.
Oh well, each to their own.
Here is a bit more information on this subject:
$ ls /sys/bus/usb/devices
1-0:1.0 3-7:1.1 5-2.1.1.2 5-2.5 6-2.3.4
2-0:1.0 3-7:1.2 5-2.1.1.2:1.0 5-2.5:1.0 6-2.3.4:1.0
3-0:1.0 4-0:1.0 5-2.1.1.2:1.1 6-0:1.0 usb1
3-3 5-0:1.0 5-2.1.1.2:1.2 6-2 usb2
3-3:1.0 5-1 5-2.1.1.2:1.3 6-2.1 usb3
3-4 5-2 5-2.3 6-2:1.0 usb4
3-4:1.0 5-2.1 5-2.3:1.0 6-2.1:1.0 usb5
3-4:1.1 5-2:1.0 5-2.3.4 6-2.1.2 usb6
3-4:1.2 5-2.1.1 5-2.3.4:1.0 6-2.1.2:1.0
3-7 5-2.1:1.0 5-2.3.4.3 6-2.3
3-7:1.0 5-2.1.1:1.0 5-2.3.4.3:2.0 6-2.3:1.0
And the missing device is 5-1 and looks like this:
$ cd /sys/bus/usb/devices/5-1
$ ls_name_value
authorized : 1
avoid_reset_quirk : 0
bcdDevice : 0200
bConfigurationValue : 1
bDeviceClass : 11
bDeviceProtocol : 00
bDeviceSubClass : 00
bmAttributes : c0
bMaxPacketSize0 : 64
bMaxPower : 0mA
bNumConfigurations : 1
bNumInterfaces : 0
busnum : 5
configuration :
descriptors : <contains non-ASCII chars>
dev : 189:526
devnum : 15
devpath : 1
devspec : (null)
idProduct : 572b
idVendor : 0483
ltm_capable : no
manufacturer : STMicroelectronics
maxchild : 0
product : STEVAL-USBC2DP Type-C to DisplayPort adapter
quirks : 0x0
removable : unknown
remove :
rx_lanes : 1
serial : 00000000002B
speed : 12
tx_lanes : 1
uevent : MAJOR=189 MINOR=526 DEVNAME=bus/usb/005/015 DEVTYPE=usb_device
DRIVER=usb PRODUCT=483/572b/200 TYPE=17/0/0 BUSNUM=005 DEVNUM=015
urbnum : 14
version : 2.01
That all looks correct.
The code in lsusb-t.c seems to assign a special meaning to "-1" devices
and there is only one of those: "5-1". And the device associated with
"5-1" is the one that does _not_ appear in the output of 'lsusb -t' but
does appear in the output of 'lsusb'.
Doug Gilbert
^ permalink raw reply
* Re: [PATCH RFC v5 1/6] ARM: pxa: Convert Spitz OHCI to GPIO descriptors
From: Bartosz Golaszewski @ 2023-10-06 6:48 UTC (permalink / raw)
To: Duje Mihanović
Cc: Daniel Mack, Haojian Zhuang, Robert Jarzmik, Russell King,
Alan Stern, Greg Kroah-Hartman, Linus Walleij, Andy Shevchenko,
Dmitry Torokhov, Mark Brown, linux-arm-kernel, linux-kernel,
linux-usb, linux-gpio, linux-input, linux-spi
In-Reply-To: <20231004-pxa-gpio-v5-1-d99ae6fceea8@skole.hr>
On Wed, Oct 4, 2023 at 4:56 PM Duje Mihanović <duje.mihanovic@skole.hr> wrote:
>
> Sharp's Spitz board still uses the legacy GPIO interface for controlling
> a GPIO pin related to the USB host controller.
>
> Convert this function to use the new GPIO descriptor interface.
>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Duje Mihanović <duje.mihanovic@skole.hr>
> ---
> arch/arm/mach-pxa/spitz.c | 13 ++++++-------
> drivers/usb/host/ohci-pxa27x.c | 7 +++++++
> 2 files changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/arch/arm/mach-pxa/spitz.c b/arch/arm/mach-pxa/spitz.c
> index cc691b199429..535e2b2e997b 100644
> --- a/arch/arm/mach-pxa/spitz.c
> +++ b/arch/arm/mach-pxa/spitz.c
> @@ -649,23 +649,22 @@ static inline void spitz_mmc_init(void) {}
> * USB Host
> ******************************************************************************/
> #if defined(CONFIG_USB_OHCI_HCD) || defined(CONFIG_USB_OHCI_HCD_MODULE)
> +GPIO_LOOKUP_SINGLE(spitz_usb_host_gpio_table, "pxa27x-ohci", "gpio-pxa",
> + SPITZ_GPIO_USB_HOST, "usb-host", GPIO_ACTIVE_LOW);
> +
> static int spitz_ohci_init(struct device *dev)
> {
> - int err;
> -
> - err = gpio_request(SPITZ_GPIO_USB_HOST, "USB_HOST");
> - if (err)
> - return err;
> + gpiod_add_lookup_table(&spitz_usb_host_gpio_table);
>
> /* Only Port 2 is connected, setup USB Port 2 Output Control Register */
> UP2OCR = UP2OCR_HXS | UP2OCR_HXOE | UP2OCR_DPPDE | UP2OCR_DMPDE;
>
> - return gpio_direction_output(SPITZ_GPIO_USB_HOST, 1);
> + return 0;
> }
>
> static void spitz_ohci_exit(struct device *dev)
> {
> - gpio_free(SPITZ_GPIO_USB_HOST);
> + gpiod_remove_lookup_table(&spitz_usb_host_gpio_table);
> }
>
> static struct pxaohci_platform_data spitz_ohci_platform_data = {
> diff --git a/drivers/usb/host/ohci-pxa27x.c b/drivers/usb/host/ohci-pxa27x.c
> index 357d9aee38a3..876842b940c0 100644
> --- a/drivers/usb/host/ohci-pxa27x.c
> +++ b/drivers/usb/host/ohci-pxa27x.c
> @@ -121,6 +121,7 @@ struct pxa27x_ohci {
> void __iomem *mmio_base;
> struct regulator *vbus[3];
> bool vbus_enabled[3];
> + struct gpio_desc *usb_host;
> };
>
> #define to_pxa27x_ohci(hcd) (struct pxa27x_ohci *)(hcd_to_ohci(hcd)->priv)
> @@ -447,6 +448,10 @@ static int ohci_hcd_pxa27x_probe(struct platform_device *pdev)
> pxa_ohci = to_pxa27x_ohci(hcd);
> pxa_ohci->clk = usb_clk;
> pxa_ohci->mmio_base = (void __iomem *)hcd->regs;
> + pxa_ohci->usb_host = gpiod_get_optional(&pdev->dev, "usb-host", GPIOD_OUT_LOW);
Any reason not to use devm_gpiod_get_optional()?
Bart
> + if (IS_ERR(pxa_ohci->usb_host))
> + return dev_err_probe(&pdev->dev, PTR_ERR(pxa_ohci->usb_host),
> + "failed to get USB host GPIO\n");
>
> for (i = 0; i < 3; ++i) {
> char name[6];
> @@ -512,6 +517,8 @@ static void ohci_hcd_pxa27x_remove(struct platform_device *pdev)
> for (i = 0; i < 3; ++i)
> pxa27x_ohci_set_vbus_power(pxa_ohci, i, false);
>
> + gpiod_put(pxa_ohci->usb_host);
> +
> usb_put_hcd(hcd);
> }
>
>
> --
> 2.42.0
>
>
^ permalink raw reply
* Re: [PATCH RFC v5 2/6] ARM: pxa: Convert Spitz LEDs to GPIO descriptors
From: Bartosz Golaszewski @ 2023-10-06 6:50 UTC (permalink / raw)
To: Duje Mihanović
Cc: Daniel Mack, Haojian Zhuang, Robert Jarzmik, Russell King,
Alan Stern, Greg Kroah-Hartman, Linus Walleij, Andy Shevchenko,
Dmitry Torokhov, Mark Brown, linux-arm-kernel, linux-kernel,
linux-usb, linux-gpio, linux-input, linux-spi
In-Reply-To: <20231004-pxa-gpio-v5-2-d99ae6fceea8@skole.hr>
On Wed, Oct 4, 2023 at 4:56 PM Duje Mihanović <duje.mihanovic@skole.hr> wrote:
>
> Sharp's Spitz board still uses the legacy GPIO interface for configuring
> its two onboard LEDs.
>
> Convert them to use the GPIO descriptor interface.
>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Duje Mihanović <duje.mihanovic@skole.hr>
> ---
> arch/arm/mach-pxa/spitz.c | 18 ++++++++++++++++--
> 1 file changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/mach-pxa/spitz.c b/arch/arm/mach-pxa/spitz.c
> index 535e2b2e997b..b6a4085e9fb0 100644
> --- a/arch/arm/mach-pxa/spitz.c
> +++ b/arch/arm/mach-pxa/spitz.c
> @@ -452,16 +452,25 @@ static inline void spitz_keys_init(void) {}
> * LEDs
> ******************************************************************************/
> #if defined(CONFIG_LEDS_GPIO) || defined(CONFIG_LEDS_GPIO_MODULE)
> +static struct gpiod_lookup_table spitz_led_gpio_table = {
> + .dev_id = "leds-gpio",
> + .table = {
> + GPIO_LOOKUP_IDX("pxa-gpio", SPITZ_GPIO_LED_ORANGE, NULL, 0,
> + GPIO_ACTIVE_HIGH),
> + GPIO_LOOKUP_IDX("pxa-gpio", SPITZ_GPIO_LED_GREEN, NULL, 1,
> + GPIO_ACTIVE_HIGH),
> + { }
> + }
> +};
> +
> static struct gpio_led spitz_gpio_leds[] = {
> {
> .name = "spitz:amber:charge",
> .default_trigger = "sharpsl-charge",
> - .gpio = SPITZ_GPIO_LED_ORANGE,
> },
> {
> .name = "spitz:green:hddactivity",
> .default_trigger = "disk-activity",
> - .gpio = SPITZ_GPIO_LED_GREEN,
> },
> };
>
> @@ -480,7 +489,12 @@ static struct platform_device spitz_led_device = {
>
> static void __init spitz_leds_init(void)
> {
> + gpiod_add_lookup_table(&spitz_led_gpio_table);
> platform_device_register(&spitz_led_device);
> + spitz_gpio_leds[0].gpiod = gpiod_get_index(&spitz_led_device.dev,
> + NULL, 0, GPIOD_ASIS);
> + spitz_gpio_leds[1].gpiod = gpiod_get_index(&spitz_led_device.dev,
> + NULL, 1, GPIOD_ASIS);
You're not using the con_id you specified in the lookup table. How
about using gpiod_get_array()?
Bart
> }
> #else
> static inline void spitz_leds_init(void) {}
>
> --
> 2.42.0
>
>
^ permalink raw reply
* Re: [PATCH RFC v5 3/6] ARM: pxa: Convert Spitz CF power control to GPIO descriptors
From: Bartosz Golaszewski @ 2023-10-06 6:59 UTC (permalink / raw)
To: Duje Mihanović
Cc: Daniel Mack, Haojian Zhuang, Robert Jarzmik, Russell King,
Alan Stern, Greg Kroah-Hartman, Linus Walleij, Andy Shevchenko,
Dmitry Torokhov, Mark Brown, linux-arm-kernel, linux-kernel,
linux-usb, linux-gpio, linux-input, linux-spi
In-Reply-To: <20231004-pxa-gpio-v5-3-d99ae6fceea8@skole.hr>
On Wed, Oct 4, 2023 at 4:56 PM Duje Mihanović <duje.mihanovic@skole.hr> wrote:
>
> Sharp's Spitz board still uses the legacy GPIO interface for controlling
> the power supply to its CF and SD card slots.
>
> Convert it to use the GPIO descriptor interface.
>
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Duje Mihanović <duje.mihanovic@skole.hr>
> ---
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
^ permalink raw reply
* Re: [PATCH RFC v5 4/6] ARM: pxa: Convert reset driver to GPIO descriptors
From: Bartosz Golaszewski @ 2023-10-06 7:07 UTC (permalink / raw)
To: Duje Mihanović
Cc: Daniel Mack, Haojian Zhuang, Robert Jarzmik, Russell King,
Alan Stern, Greg Kroah-Hartman, Linus Walleij, Andy Shevchenko,
Dmitry Torokhov, Mark Brown, linux-arm-kernel, linux-kernel,
linux-usb, linux-gpio, linux-input, linux-spi
In-Reply-To: <20231004-pxa-gpio-v5-4-d99ae6fceea8@skole.hr>
On Wed, Oct 4, 2023 at 4:56 PM Duje Mihanović <duje.mihanovic@skole.hr> wrote:
>
> The PXA reset driver still uses the legacy GPIO interface for
> configuring and asserting the reset pin.
>
> Convert it to use the GPIO descriptor interface.
>
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Duje Mihanović <duje.mihanovic@skole.hr>
> ---
> arch/arm/mach-pxa/reset.c | 39 +++++++++++++--------------------------
> arch/arm/mach-pxa/reset.h | 3 +--
> arch/arm/mach-pxa/spitz.c | 6 +++++-
> 3 files changed, 19 insertions(+), 29 deletions(-)
>
> diff --git a/arch/arm/mach-pxa/reset.c b/arch/arm/mach-pxa/reset.c
> index 27293549f8ad..08bc104b9411 100644
> --- a/arch/arm/mach-pxa/reset.c
> +++ b/arch/arm/mach-pxa/reset.c
> @@ -2,7 +2,7 @@
> #include <linux/kernel.h>
> #include <linux/module.h>
> #include <linux/delay.h>
> -#include <linux/gpio.h>
> +#include <linux/gpio/consumer.h>
> #include <linux/io.h>
> #include <asm/proc-fns.h>
> #include <asm/system_misc.h>
> @@ -14,33 +14,20 @@
>
> static void do_hw_reset(void);
>
> -static int reset_gpio = -1;
> +static struct gpio_desc *reset_gpio;
>
> -int init_gpio_reset(int gpio, int output, int level)
> +int init_gpio_reset(int output, int level)
> {
> - int rc;
> -
> - rc = gpio_request(gpio, "reset generator");
> - if (rc) {
> - printk(KERN_ERR "Can't request reset_gpio\n");
> - goto out;
> + reset_gpio = gpiod_get(NULL, "reset", GPIOD_ASIS);
> + if (IS_ERR(reset_gpio)) {
> + pr_err("Can't request reset_gpio: %pe\n", reset_gpio);
> + return PTR_ERR(reset_gpio);
> }
>
> if (output)
> - rc = gpio_direction_output(gpio, level);
> + return gpiod_direction_output(reset_gpio, level);
> else
> - rc = gpio_direction_input(gpio);
> - if (rc) {
> - printk(KERN_ERR "Can't configure reset_gpio\n");
> - gpio_free(gpio);
> - goto out;
> - }
> -
> -out:
> - if (!rc)
> - reset_gpio = gpio;
> -
> - return rc;
> + return gpiod_direction_input(reset_gpio);
> }
>
> /*
> @@ -50,16 +37,16 @@ int init_gpio_reset(int gpio, int output, int level)
> */
> static void do_gpio_reset(void)
> {
> - BUG_ON(reset_gpio == -1);
> + BUG_ON(IS_ERR(reset_gpio));
Crashing the kernel on a GPIO error? I guess it just keeps the old
behavior but still...
>
> /* drive it low */
> - gpio_direction_output(reset_gpio, 0);
> + gpiod_direction_output(reset_gpio, 0);
> mdelay(2);
> /* rising edge or drive high */
> - gpio_set_value(reset_gpio, 1);
> + gpiod_set_value(reset_gpio, 1);
> mdelay(2);
> /* falling edge */
> - gpio_set_value(reset_gpio, 0);
> + gpiod_set_value(reset_gpio, 0);
>
> /* give it some time */
> mdelay(10);
> diff --git a/arch/arm/mach-pxa/reset.h b/arch/arm/mach-pxa/reset.h
> index 963dd190bc13..5864f61a0e94 100644
> --- a/arch/arm/mach-pxa/reset.h
> +++ b/arch/arm/mach-pxa/reset.h
> @@ -13,10 +13,9 @@ extern void pxa_register_wdt(unsigned int reset_status);
>
> /**
> * init_gpio_reset() - register GPIO as reset generator
> - * @gpio: gpio nr
> * @output: set gpio as output instead of input during normal work
> * @level: output level
> */
> -extern int init_gpio_reset(int gpio, int output, int level);
> +extern int init_gpio_reset(int output, int level);
>
> #endif /* __ASM_ARCH_RESET_H */
> diff --git a/arch/arm/mach-pxa/spitz.c b/arch/arm/mach-pxa/spitz.c
> index 965354e64c68..26ec29c9cd1b 100644
> --- a/arch/arm/mach-pxa/spitz.c
> +++ b/arch/arm/mach-pxa/spitz.c
> @@ -1024,9 +1024,13 @@ static void spitz_restart(enum reboot_mode mode, const char *cmd)
> spitz_poweroff();
> }
>
> +GPIO_LOOKUP_SINGLE(spitz_reset_gpio_table, NULL, "pxa-gpio",
> + SPITZ_GPIO_ON_RESET, "reset", GPIO_ACTIVE_HIGH);
> +
> static void __init spitz_init(void)
> {
> - init_gpio_reset(SPITZ_GPIO_ON_RESET, 1, 0);
> + gpiod_add_lookup_table(&spitz_reset_gpio_table);
> + init_gpio_reset(1, 0);
> pm_power_off = spitz_poweroff;
>
> PMCR = 0x00;
>
> --
> 2.42.0
>
>
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
^ permalink raw reply
* Re: [PATCH RFC v5 5/6] ARM: pxa: Convert gumstix Bluetooth to GPIO descriptors
From: Bartosz Golaszewski @ 2023-10-06 7:10 UTC (permalink / raw)
To: Duje Mihanović
Cc: Daniel Mack, Haojian Zhuang, Robert Jarzmik, Russell King,
Alan Stern, Greg Kroah-Hartman, Linus Walleij, Andy Shevchenko,
Dmitry Torokhov, Mark Brown, linux-arm-kernel, linux-kernel,
linux-usb, linux-gpio, linux-input, linux-spi
In-Reply-To: <20231004-pxa-gpio-v5-5-d99ae6fceea8@skole.hr>
On Wed, Oct 4, 2023 at 4:56 PM Duje Mihanović <duje.mihanovic@skole.hr> wrote:
>
> Gumstix still uses the legacy GPIO interface for resetting the Bluetooth
> device.
>
> Convert it to use the GPIO descriptor interface.
>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Duje Mihanović <duje.mihanovic@skole.hr>
> ---
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
^ permalink raw reply
* Re: [PATCH RFC v5 6/6] input: ads7846: Move wait_for_sync() logic to driver
From: Bartosz Golaszewski @ 2023-10-06 7:11 UTC (permalink / raw)
To: Duje Mihanović
Cc: Daniel Mack, Haojian Zhuang, Robert Jarzmik, Russell King,
Alan Stern, Greg Kroah-Hartman, Linus Walleij, Andy Shevchenko,
Dmitry Torokhov, Mark Brown, linux-arm-kernel, linux-kernel,
linux-usb, linux-gpio, linux-input, linux-spi
In-Reply-To: <20231004-pxa-gpio-v5-6-d99ae6fceea8@skole.hr>
On Wed, Oct 4, 2023 at 4:56 PM Duje Mihanović <duje.mihanovic@skole.hr> wrote:
>
> If this code is left in the board file, the sync GPIO would have to be
> separated into another lookup table during conversion to the GPIO
> descriptor API (which is also done in this patch).
>
> The only user of this code (Sharp Spitz) is also converted in this
> patch.
>
> Suggested-by: Linus Walleij <linus.walleij@linaro.org>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> Acked-by: Mark Brown <broonie@kernel.org>
> Signed-off-by: Duje Mihanović <duje.mihanovic@skole.hr>
> ---
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
^ permalink raw reply
* [PATCH] usb: misc: onboard_usb_hub: extend gl3510 reset duration
From: Jerome Brunet @ 2023-10-06 10:10 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: Jerome Brunet, linux-kernel, linux-usb, Neil Armstrong
Initial tests with the gl3510 has been done on libretech aml-a311d-cc.
A 50us reset was fine as long as the hub node was under the usb phy node it
DT. DT schema does not allow that. Moving the hub under the dwc3 controller
caused issues, such as:
onboard-usb-hub 1-1: Failed to suspend device, error -32
onboard-usb-hub 1-1: can't set config #1, error -71
onboard-usb-hub 1-1: Failed to suspend device, error -32
onboard-usb-hub 1-1: USB disconnect, device number 2
Extending the reset duration solves the problem.
Since there is no documentation available for this hub, it is difficult to
know the actual required reset duration. 200us seems to work fine so far.
Suggested-by: Neil Armstrong <neil.armstrong@linaro.org>
Fixes: 65009ccf7e8f ("usb: misc: onboard_usb_hub: add Genesys Logic gl3510 hub support")
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
drivers/usb/misc/onboard_usb_hub.h | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/misc/onboard_usb_hub.h b/drivers/usb/misc/onboard_usb_hub.h
index a9e2f6023c1c..38de22452963 100644
--- a/drivers/usb/misc/onboard_usb_hub.h
+++ b/drivers/usb/misc/onboard_usb_hub.h
@@ -31,6 +31,11 @@ static const struct onboard_hub_pdata cypress_hx3_data = {
.num_supplies = 2,
};
+static const struct onboard_hub_pdata genesys_gl3510_data = {
+ .reset_us = 200,
+ .num_supplies = 1,
+};
+
static const struct onboard_hub_pdata genesys_gl850g_data = {
.reset_us = 3,
.num_supplies = 1,
@@ -56,7 +61,7 @@ static const struct of_device_id onboard_hub_match[] = {
{ .compatible = "usb5e3,608", .data = &genesys_gl850g_data, },
{ .compatible = "usb5e3,610", .data = &genesys_gl852g_data, },
{ .compatible = "usb5e3,620", .data = &genesys_gl852g_data, },
- { .compatible = "usb5e3,626", .data = &genesys_gl852g_data, },
+ { .compatible = "usb5e3,626", .data = &genesys_gl3510_data, },
{ .compatible = "usbbda,411", .data = &realtek_rts5411_data, },
{ .compatible = "usbbda,5411", .data = &realtek_rts5411_data, },
{ .compatible = "usbbda,414", .data = &realtek_rts5411_data, },
--
2.40.1
^ permalink raw reply related
* Re: [syzbot] [pvrusb2?] [usb?] KASAN: slab-use-after-free Read in pvr2_context_set_notify
From: Ricardo B. Marliere @ 2023-10-06 12:17 UTC (permalink / raw)
To: syzbot
Cc: isely, linux-kernel, linux-media, linux-usb, mchehab, pvrusb2,
syzkaller-bugs
In-Reply-To: <gugiuvjgpoogf3k5cm4px4jwevg5torsu3d7afbbhvnrxho4zu@wkcxeb5sr5ez>
#syz test: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git 1053c4a4b8fcbd28386e80347e7c82d4d617e352
^ permalink raw reply
* Re: [syzbot] [pvrusb2?] KASAN: slab-use-after-free Read in pvr2_context_set_notify
From: syzbot @ 2023-10-06 12:42 UTC (permalink / raw)
To: isely, linux-kernel, linux-media, linux-usb, mchehab, pvrusb2,
ricardo, syzkaller-bugs
In-Reply-To: <cb6fm6c65rqbk6hzjii5bqanscy7njfu5k7nnpe4doxytshqpf@ulv5noywsnlv>
Hello,
syzbot has tested the proposed patch but the reproducer is still triggering an issue:
KASAN: slab-use-after-free Read in pvr2_context_set_notify
pvrusb2: Important functionality might not be entirely working.
pvrusb2: Please consider contacting the driver author to help with further stabilization of the driver.
pvrusb2: **********
usb 1-1: USB disconnect, device number 3
pvrusb2: Device being rendered inoperable
==================================================================
BUG: KASAN: slab-use-after-free in pvr2_context_set_notify+0x2c4/0x310 drivers/media/usb/pvrusb2/pvrusb2-context.c:35
Read of size 4 at addr ffff88812c3e68d8 by task kworker/0:2/699
CPU: 0 PID: 699 Comm: kworker/0:2 Not tainted 6.6.0-rc4-syzkaller-00064-g1053c4a4b8fc #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 09/06/2023
Workqueue: usb_hub_wq hub_event
Call Trace:
<TASK>
__dump_stack lib/dump_stack.c:88 [inline]
dump_stack_lvl+0xd9/0x1b0 lib/dump_stack.c:106
print_address_description mm/kasan/report.c:364 [inline]
print_report+0xc4/0x620 mm/kasan/report.c:475
kasan_report+0xda/0x110 mm/kasan/report.c:588
pvr2_context_set_notify+0x2c4/0x310 drivers/media/usb/pvrusb2/pvrusb2-context.c:35
pvr_disconnect+0x80/0xf0 drivers/media/usb/pvrusb2/pvrusb2-main.c:79
usb_unbind_interface+0x1dd/0x8d0 drivers/usb/core/driver.c:458
device_remove drivers/base/dd.c:569 [inline]
device_remove+0x11f/0x170 drivers/base/dd.c:561
__device_release_driver drivers/base/dd.c:1272 [inline]
device_release_driver_internal+0x44a/0x610 drivers/base/dd.c:1295
bus_remove_device+0x22c/0x420 drivers/base/bus.c:574
device_del+0x39a/0xa50 drivers/base/core.c:3813
usb_disable_device+0x36c/0x7f0 drivers/usb/core/message.c:1416
usb_disconnect+0x2e1/0x890 drivers/usb/core/hub.c:2252
hub_port_connect drivers/usb/core/hub.c:5280 [inline]
hub_port_connect_change drivers/usb/core/hub.c:5580 [inline]
port_event drivers/usb/core/hub.c:5740 [inline]
hub_event+0x1be0/0x4f30 drivers/usb/core/hub.c:5822
process_one_work+0x884/0x15c0 kernel/workqueue.c:2630
process_scheduled_works kernel/workqueue.c:2703 [inline]
worker_thread+0x8b9/0x1290 kernel/workqueue.c:2784
kthread+0x33c/0x440 kernel/kthread.c:388
ret_from_fork+0x45/0x80 arch/x86/kernel/process.c:147
ret_from_fork_asm+0x11/0x20 arch/x86/entry/entry_64.S:304
</TASK>
Allocated by task 699:
kasan_save_stack+0x33/0x50 mm/kasan/common.c:45
kasan_set_track+0x25/0x30 mm/kasan/common.c:52
____kasan_kmalloc mm/kasan/common.c:374 [inline]
__kasan_kmalloc+0x87/0x90 mm/kasan/common.c:383
kmalloc include/linux/slab.h:599 [inline]
kzalloc include/linux/slab.h:720 [inline]
pvr2_context_create+0x53/0x2a0 drivers/media/usb/pvrusb2/pvrusb2-context.c:207
pvr_probe+0x25/0xe0 drivers/media/usb/pvrusb2/pvrusb2-main.c:54
usb_probe_interface+0x307/0x930 drivers/usb/core/driver.c:396
call_driver_probe drivers/base/dd.c:579 [inline]
really_probe+0x234/0xc90 drivers/base/dd.c:658
__driver_probe_device+0x1de/0x4b0 drivers/base/dd.c:800
driver_probe_device+0x4c/0x1a0 drivers/base/dd.c:830
__device_attach_driver+0x1d4/0x300 drivers/base/dd.c:958
bus_for_each_drv+0x157/0x1d0 drivers/base/bus.c:457
__device_attach+0x1e8/0x4b0 drivers/base/dd.c:1030
bus_probe_device+0x17c/0x1c0 drivers/base/bus.c:532
device_add+0x117e/0x1aa0 drivers/base/core.c:3624
usb_set_configuration+0x10cb/0x1c40 drivers/usb/core/message.c:2207
usb_generic_driver_probe+0xca/0x130 drivers/usb/core/generic.c:238
usb_probe_device+0xda/0x2c0 drivers/usb/core/driver.c:293
call_driver_probe drivers/base/dd.c:579 [inline]
really_probe+0x234/0xc90 drivers/base/dd.c:658
__driver_probe_device+0x1de/0x4b0 drivers/base/dd.c:800
driver_probe_device+0x4c/0x1a0 drivers/base/dd.c:830
__device_attach_driver+0x1d4/0x300 drivers/base/dd.c:958
bus_for_each_drv+0x157/0x1d0 drivers/base/bus.c:457
__device_attach+0x1e8/0x4b0 drivers/base/dd.c:1030
bus_probe_device+0x17c/0x1c0 drivers/base/bus.c:532
device_add+0x117e/0x1aa0 drivers/base/core.c:3624
usb_new_device+0xd80/0x1960 drivers/usb/core/hub.c:2589
hub_port_connect drivers/usb/core/hub.c:5440 [inline]
hub_port_connect_change drivers/usb/core/hub.c:5580 [inline]
port_event drivers/usb/core/hub.c:5740 [inline]
hub_event+0x2e62/0x4f30 drivers/usb/core/hub.c:5822
process_one_work+0x884/0x15c0 kernel/workqueue.c:2630
process_scheduled_works kernel/workqueue.c:2703 [inline]
worker_thread+0x8b9/0x1290 kernel/workqueue.c:2784
kthread+0x33c/0x440 kernel/kthread.c:388
ret_from_fork+0x45/0x80 arch/x86/kernel/process.c:147
ret_from_fork_asm+0x11/0x20 arch/x86/entry/entry_64.S:304
Freed by task 902:
kasan_save_stack+0x33/0x50 mm/kasan/common.c:45
kasan_set_track+0x25/0x30 mm/kasan/common.c:52
kasan_save_free_info+0x2b/0x40 mm/kasan/generic.c:522
____kasan_slab_free mm/kasan/common.c:236 [inline]
____kasan_slab_free+0x13c/0x190 mm/kasan/common.c:200
kasan_slab_free include/linux/kasan.h:164 [inline]
slab_free_hook mm/slub.c:1800 [inline]
slab_free_freelist_hook mm/slub.c:1826 [inline]
slab_free mm/slub.c:3809 [inline]
__kmem_cache_free+0xff/0x330 mm/slub.c:3822
pvr2_context_check drivers/media/usb/pvrusb2/pvrusb2-context.c:137 [inline]
pvr2_context_thread_func+0x69d/0x960 drivers/media/usb/pvrusb2/pvrusb2-context.c:158
kthread+0x33c/0x440 kernel/kthread.c:388
ret_from_fork+0x45/0x80 arch/x86/kernel/process.c:147
ret_from_fork_asm+0x11/0x20 arch/x86/entry/entry_64.S:304
The buggy address belongs to the object at ffff88812c3e6800
which belongs to the cache kmalloc-256 of size 256
The buggy address is located 216 bytes inside of
freed 256-byte region [ffff88812c3e6800, ffff88812c3e6900)
The buggy address belongs to the physical page:
page:ffffea0004b0f980 refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x12c3e6
head:ffffea0004b0f980 order:1 entire_mapcount:0 nr_pages_mapped:0 pincount:0
flags: 0x200000000000840(slab|head|node=0|zone=2)
page_type: 0xffffffff()
raw: 0200000000000840 ffff888100041b40 ffffea0004722a80 dead000000000004
raw: 0000000000000000 0000000080100010 00000001ffffffff 0000000000000000
page dumped because: kasan: bad access detected
page_owner tracks the page as allocated
page last allocated via order 1, migratetype Unmovable, gfp_mask 0x1d20c0(__GFP_IO|__GFP_FS|__GFP_NOWARN|__GFP_NORETRY|__GFP_COMP|__GFP_NOMEMALLOC|__GFP_HARDWALL), pid 699, tgid 699 (kworker/0:2), ts 149486728006, free_ts 149464487231
set_page_owner include/linux/page_owner.h:31 [inline]
post_alloc_hook+0x2cf/0x340 mm/page_alloc.c:1536
prep_new_page mm/page_alloc.c:1543 [inline]
get_page_from_freelist+0x10e1/0x2fd0 mm/page_alloc.c:3170
__alloc_pages+0x1d0/0x4a0 mm/page_alloc.c:4426
alloc_pages+0x1a9/0x270 mm/mempolicy.c:2297
alloc_slab_page mm/slub.c:1870 [inline]
allocate_slab+0x251/0x380 mm/slub.c:2017
new_slab mm/slub.c:2070 [inline]
___slab_alloc+0x8c7/0x1580 mm/slub.c:3223
__slab_alloc.constprop.0+0x56/0xa0 mm/slub.c:3322
__slab_alloc_node mm/slub.c:3375 [inline]
slab_alloc_node mm/slub.c:3468 [inline]
__kmem_cache_alloc_node+0x12c/0x310 mm/slub.c:3517
kmalloc_trace+0x25/0xe0 mm/slab_common.c:1114
kmalloc include/linux/slab.h:599 [inline]
kzalloc include/linux/slab.h:720 [inline]
pvr2_context_create+0x53/0x2a0 drivers/media/usb/pvrusb2/pvrusb2-context.c:207
pvr_probe+0x25/0xe0 drivers/media/usb/pvrusb2/pvrusb2-main.c:54
usb_probe_interface+0x307/0x930 drivers/usb/core/driver.c:396
call_driver_probe drivers/base/dd.c:579 [inline]
really_probe+0x234/0xc90 drivers/base/dd.c:658
__driver_probe_device+0x1de/0x4b0 drivers/base/dd.c:800
driver_probe_device+0x4c/0x1a0 drivers/base/dd.c:830
__device_attach_driver+0x1d4/0x300 drivers/base/dd.c:958
page last free stack trace:
reset_page_owner include/linux/page_owner.h:24 [inline]
free_pages_prepare mm/page_alloc.c:1136 [inline]
free_unref_page_prepare+0x460/0xa20 mm/page_alloc.c:2312
free_unref_page+0x33/0x2c0 mm/page_alloc.c:2405
qlink_free mm/kasan/quarantine.c:166 [inline]
qlist_free_all+0x6a/0x170 mm/kasan/quarantine.c:185
kasan_quarantine_reduce+0x18e/0x1d0 mm/kasan/quarantine.c:292
__kasan_slab_alloc+0x4a/0x70 mm/kasan/common.c:305
kasan_slab_alloc include/linux/kasan.h:188 [inline]
slab_post_alloc_hook mm/slab.h:762 [inline]
slab_alloc_node mm/slub.c:3478 [inline]
__kmem_cache_alloc_node+0x190/0x310 mm/slub.c:3517
__do_kmalloc_node mm/slab_common.c:1022 [inline]
__kmalloc+0x4f/0x100 mm/slab_common.c:1036
kmalloc include/linux/slab.h:603 [inline]
tomoyo_realpath_from_path+0xb9/0x710 security/tomoyo/realpath.c:251
tomoyo_get_realpath security/tomoyo/file.c:151 [inline]
tomoyo_path_number_perm+0x241/0x580 security/tomoyo/file.c:723
security_file_ioctl+0x72/0xb0 security/security.c:2647
__do_sys_ioctl fs/ioctl.c:865 [inline]
__se_sys_ioctl fs/ioctl.c:857 [inline]
__x64_sys_ioctl+0xbb/0x210 fs/ioctl.c:857
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x38/0xb0 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x63/0xcd
Memory state around the buggy address:
ffff88812c3e6780: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
ffff88812c3e6800: fa fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
>ffff88812c3e6880: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
^
ffff88812c3e6900: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
ffff88812c3e6980: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
==================================================================
Tested on:
commit: 1053c4a4 Revert "usb: gadget: uvc: stop pump thread on..
git tree: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git
console output: https://syzkaller.appspot.com/x/log.txt?x=1472423a680000
kernel config: https://syzkaller.appspot.com/x/.config?x=a5e01246f94ceed9
dashboard link: https://syzkaller.appspot.com/bug?extid=621409285c4156a009b3
compiler: gcc (Debian 12.2.0-14) 12.2.0, GNU ld (GNU Binutils for Debian) 2.40
Note: no patches were applied.
^ permalink raw reply
* [RFC PATCH 0/6] usb-storage,uas,scsi: Support OPAL commands on USB attached devices.
From: Milan Broz @ 2023-10-06 12:54 UTC (permalink / raw)
To: linux-usb
Cc: usb-storage, linux-scsi, linux-block, stern, oneukum,
jonathan.derrick, Milan Broz
This patchset adds support for OPAL commands (self-encrypted drives)
through USB-attached storage (usb-storage and UAS drivers).
1) Patches 1-4 only add support for 64-bit quirks for USB storage
(unfortunately, USB device info can be 32-bit on 32-bit platforms,
and we are out of space for flags now).
2) Patches 5-6 enable OPAL commands on USB device and also adds
an ATA-12 pass-thru wrapper to support OPAL even on devices that
do not support SCSI SECURITY IN/OUT commands.
ATA-12 is already used by sedutils directly; this patch makes
internal kernel OPAL ioctl work with ATA-12 too.
As patch 6 introduced a new USB quirk that overflows 32-bit,
I posted all patches together - but logically, these solve two
separate issues.
More info
1) 64bit USB storage quirk flags
The quirks are transferred through the device info value, which
is unsigned long (and as a part of USB infrastructure, it cannot
be changed).
After discussion on USB list, I used high bit as an indicator
that the values need to be translated/unpacked to 64bit
(while lower values are used directly).
This is implemented through a host-compiled program that
generates device tables and translation function.
As both usb-storage and UAS drivers share a lot of headers and
definitions, we need to generate separate files for usb-storage,
UAS and flags translation function.
(I also tried to use a statically generated array for flags,
but this increased the size of drivers significantly, and
the code was quite ugly...)
2) Support for OPAL on USB attached storage.
The main support for OPAL on USB-attached storage is
straightforward. The patch 6
- enables SCSI security flag for USB mass storage and UAS device
by default.
- adds an optional wrapper to the SCSI layer for the ATA-12
pass-thru command as an alternative if SECURITY IN/OUT
is unavailable.
During device detection, these steps are then done:
1) USB driver (mass-storage, UAS) enables security driver flag
by default if not disabled by quirk
2) SCSI device enumerates SECURITY IN/OUT support. If detected,
SECURITY ON/OUT wrapper is used (as in the current code).
If not, the new ATA12 pass-thru wrapper is used instead.
3) SED OPAL code tries OPAL discovery command for the device.
If it receives correct reply, OPAL is enabled for the device.
Enabling support may uncover many issues, as OPAL-locked devices often
tend to generate errors on the locked range.
Anyway, cryptsetup will soon support OPAL devices, and I think support
for USB devices is a nice feature that enables users to unlock drives
even if they are attached through USB adapters.
But also, there are bugs in firmware, so I added a quirk flag that can
disable security commands for particular devices.
The last patch uses this quirk for Realtek 9210, which seems to support
OPAL commands, but after configuring OPAL locking range, it also sets
the write-protected flag for the whole device.
This is perhaps a bug in firmware (all versions I tried), and I will
report that later to Realtek.
Milan Broz (6):
usb-storage: remove UNUSUAL_VENDOR_INTF macro
usb-storage: make internal quirks flags 64bit
usb-storage: use fflags index only in usb-storage driver
usb-storage,uas: use host helper to generate driver info
usb-storage,uas,scsi: allow to pass through security commands (OPAL)
usb-storage,uas: Disable security commands (OPAL) for RT9210 chip
family
drivers/scsi/sd.c | 33 ++++-
drivers/usb/storage/Makefile | 25 ++++
drivers/usb/storage/alauda.c | 2 +-
drivers/usb/storage/cypress_atacb.c | 2 +-
drivers/usb/storage/datafab.c | 2 +-
drivers/usb/storage/ene_ub6250.c | 2 +-
drivers/usb/storage/freecom.c | 2 +-
drivers/usb/storage/isd200.c | 2 +-
drivers/usb/storage/jumpshot.c | 2 +-
drivers/usb/storage/karma.c | 2 +-
drivers/usb/storage/mkflags.c | 212 ++++++++++++++++++++++++++++
drivers/usb/storage/onetouch.c | 2 +-
drivers/usb/storage/realtek_cr.c | 2 +-
drivers/usb/storage/scsiglue.c | 4 +
drivers/usb/storage/sddr09.c | 2 +-
drivers/usb/storage/sddr55.c | 2 +-
drivers/usb/storage/shuttle_usbat.c | 2 +-
drivers/usb/storage/uas-detect.h | 4 +-
drivers/usb/storage/uas.c | 26 ++--
drivers/usb/storage/unusual_devs.h | 11 ++
drivers/usb/storage/unusual_uas.h | 11 ++
drivers/usb/storage/usb.c | 42 +++---
drivers/usb/storage/usb.h | 7 +-
drivers/usb/storage/usual-tables.c | 38 +----
include/linux/usb_usual.h | 2 +
25 files changed, 346 insertions(+), 95 deletions(-)
create mode 100644 drivers/usb/storage/mkflags.c
--
2.42.0
^ permalink raw reply
* [RFC PATCH 1/6] usb-storage: remove UNUSUAL_VENDOR_INTF macro
From: Milan Broz @ 2023-10-06 12:54 UTC (permalink / raw)
To: linux-usb
Cc: usb-storage, linux-scsi, linux-block, stern, oneukum,
jonathan.derrick, Milan Broz
In-Reply-To: <20231006125445.122380-1-gmazyland@gmail.com>
This patch removes macro that was used only
by commit that was reverted in
commit ab4b71644a26d1ab92b987b2fd30e17c25e89f85
USB: storage: fix Huawei mode switching regression
Signed-off-by: Milan Broz <gmazyland@gmail.com>
---
drivers/usb/storage/usb.c | 12 ------------
drivers/usb/storage/usual-tables.c | 15 ---------------
2 files changed, 27 deletions(-)
diff --git a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c
index 7b36a3334fb3..bb1fbeddc5aa 100644
--- a/drivers/usb/storage/usb.c
+++ b/drivers/usb/storage/usb.c
@@ -110,17 +110,6 @@ MODULE_PARM_DESC(quirks, "supplemental list of device IDs and their quirks");
.useTransport = use_transport, \
}
-#define UNUSUAL_VENDOR_INTF(idVendor, cl, sc, pr, \
- vendor_name, product_name, use_protocol, use_transport, \
- init_function, Flags) \
-{ \
- .vendorName = vendor_name, \
- .productName = product_name, \
- .useProtocol = use_protocol, \
- .useTransport = use_transport, \
- .initFunction = init_function, \
-}
-
static const struct us_unusual_dev us_unusual_dev_list[] = {
# include "unusual_devs.h"
{ } /* Terminating entry */
@@ -132,7 +121,6 @@ static const struct us_unusual_dev for_dynamic_ids =
#undef UNUSUAL_DEV
#undef COMPLIANT_DEV
#undef USUAL_DEV
-#undef UNUSUAL_VENDOR_INTF
#ifdef CONFIG_LOCKDEP
diff --git a/drivers/usb/storage/usual-tables.c b/drivers/usb/storage/usual-tables.c
index 529512827d8f..b3c3ea04c11c 100644
--- a/drivers/usb/storage/usual-tables.c
+++ b/drivers/usb/storage/usual-tables.c
@@ -26,20 +26,6 @@
#define USUAL_DEV(useProto, useTrans) \
{ USB_INTERFACE_INFO(USB_CLASS_MASS_STORAGE, useProto, useTrans) }
-/* Define the device is matched with Vendor ID and interface descriptors */
-#define UNUSUAL_VENDOR_INTF(id_vendor, cl, sc, pr, \
- vendorName, productName, useProtocol, useTransport, \
- initFunction, flags) \
-{ \
- .match_flags = USB_DEVICE_ID_MATCH_INT_INFO \
- | USB_DEVICE_ID_MATCH_VENDOR, \
- .idVendor = (id_vendor), \
- .bInterfaceClass = (cl), \
- .bInterfaceSubClass = (sc), \
- .bInterfaceProtocol = (pr), \
- .driver_info = (flags) \
-}
-
const struct usb_device_id usb_storage_usb_ids[] = {
# include "unusual_devs.h"
{ } /* Terminating entry */
@@ -49,7 +35,6 @@ MODULE_DEVICE_TABLE(usb, usb_storage_usb_ids);
#undef UNUSUAL_DEV
#undef COMPLIANT_DEV
#undef USUAL_DEV
-#undef UNUSUAL_VENDOR_INTF
/*
* The table of devices to ignore
--
2.42.0
^ permalink raw reply related
* [RFC PATCH 2/6] usb-storage: make internal quirks flags 64bit
From: Milan Broz @ 2023-10-06 12:54 UTC (permalink / raw)
To: linux-usb
Cc: usb-storage, linux-scsi, linux-block, stern, oneukum,
jonathan.derrick, Milan Broz
In-Reply-To: <20231006125445.122380-1-gmazyland@gmail.com>
Switch internal usb-storage quirk value to 64-bit as quirks currently
already use all 32 bits.
(Following patches are needed to properly use driver_info
for 64-bit value.)
Signed-off-by: Milan Broz <gmazyland@gmail.com>
---
drivers/usb/storage/uas-detect.h | 4 ++--
drivers/usb/storage/uas.c | 4 ++--
drivers/usb/storage/usb.c | 8 ++++----
drivers/usb/storage/usb.h | 4 ++--
drivers/usb/storage/usual-tables.c | 2 +-
5 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/drivers/usb/storage/uas-detect.h b/drivers/usb/storage/uas-detect.h
index d73282c0ec50..4d3b49e5b87a 100644
--- a/drivers/usb/storage/uas-detect.h
+++ b/drivers/usb/storage/uas-detect.h
@@ -54,12 +54,12 @@ static int uas_find_endpoints(struct usb_host_interface *alt,
static int uas_use_uas_driver(struct usb_interface *intf,
const struct usb_device_id *id,
- unsigned long *flags_ret)
+ u64 *flags_ret)
{
struct usb_host_endpoint *eps[4] = { };
struct usb_device *udev = interface_to_usbdev(intf);
struct usb_hcd *hcd = bus_to_hcd(udev->bus);
- unsigned long flags = id->driver_info;
+ u64 flags = id->driver_info;
struct usb_host_interface *alt;
int r;
diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index 2583ee9815c5..696bb0b23599 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -37,7 +37,7 @@ struct uas_dev_info {
struct usb_anchor cmd_urbs;
struct usb_anchor sense_urbs;
struct usb_anchor data_urbs;
- unsigned long flags;
+ u64 flags;
int qdepth, resetting;
unsigned cmd_pipe, status_pipe, data_in_pipe, data_out_pipe;
unsigned use_streams:1;
@@ -988,7 +988,7 @@ static int uas_probe(struct usb_interface *intf, const struct usb_device_id *id)
struct Scsi_Host *shost = NULL;
struct uas_dev_info *devinfo;
struct usb_device *udev = interface_to_usbdev(intf);
- unsigned long dev_flags;
+ u64 dev_flags;
if (!uas_use_uas_driver(intf, id, &dev_flags))
return -ENODEV;
diff --git a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c
index bb1fbeddc5aa..d1ad6a2509ab 100644
--- a/drivers/usb/storage/usb.c
+++ b/drivers/usb/storage/usb.c
@@ -460,13 +460,13 @@ static int associate_dev(struct us_data *us, struct usb_interface *intf)
#define TOLOWER(x) ((x) | 0x20)
/* Adjust device flags based on the "quirks=" module parameter */
-void usb_stor_adjust_quirks(struct usb_device *udev, unsigned long *fflags)
+void usb_stor_adjust_quirks(struct usb_device *udev, u64 *fflags)
{
char *p;
u16 vid = le16_to_cpu(udev->descriptor.idVendor);
u16 pid = le16_to_cpu(udev->descriptor.idProduct);
- unsigned f = 0;
- unsigned int mask = (US_FL_SANE_SENSE | US_FL_BAD_SENSE |
+ u64 f = 0;
+ u64 mask = (US_FL_SANE_SENSE | US_FL_BAD_SENSE |
US_FL_FIX_CAPACITY | US_FL_IGNORE_UAS |
US_FL_CAPACITY_HEURISTICS | US_FL_IGNORE_DEVICE |
US_FL_NOT_LOCKABLE | US_FL_MAX_SECTORS_64 |
@@ -605,7 +605,7 @@ static int get_device_info(struct us_data *us, const struct usb_device_id *id,
us->fflags &= ~US_FL_GO_SLOW;
if (us->fflags)
- dev_info(pdev, "Quirks match for vid %04x pid %04x: %lx\n",
+ dev_info(pdev, "Quirks match for vid %04x pid %04x: %llx\n",
le16_to_cpu(dev->descriptor.idVendor),
le16_to_cpu(dev->descriptor.idProduct),
us->fflags);
diff --git a/drivers/usb/storage/usb.h b/drivers/usb/storage/usb.h
index fd3f32670873..97c6196d639b 100644
--- a/drivers/usb/storage/usb.h
+++ b/drivers/usb/storage/usb.h
@@ -95,7 +95,7 @@ struct us_data {
struct usb_interface *pusb_intf; /* this interface */
const struct us_unusual_dev *unusual_dev;
/* device-filter entry */
- unsigned long fflags; /* fixed flags from filter */
+ u64 fflags; /* fixed flags from filter */
unsigned long dflags; /* dynamic atomic bitflags */
unsigned int send_bulk_pipe; /* cached pipe values */
unsigned int recv_bulk_pipe;
@@ -192,7 +192,7 @@ extern int usb_stor_probe2(struct us_data *us);
extern void usb_stor_disconnect(struct usb_interface *intf);
extern void usb_stor_adjust_quirks(struct usb_device *dev,
- unsigned long *fflags);
+ u64 *fflags);
#define module_usb_stor_driver(__driver, __sht, __name) \
static int __init __driver##_init(void) \
diff --git a/drivers/usb/storage/usual-tables.c b/drivers/usb/storage/usual-tables.c
index b3c3ea04c11c..a26029e43dfd 100644
--- a/drivers/usb/storage/usual-tables.c
+++ b/drivers/usb/storage/usual-tables.c
@@ -19,7 +19,7 @@
vendorName, productName, useProtocol, useTransport, \
initFunction, flags) \
{ USB_DEVICE_VER(id_vendor, id_product, bcdDeviceMin, bcdDeviceMax), \
- .driver_info = (flags) }
+ .driver_info = (kernel_ulong_t)(flags) }
#define COMPLIANT_DEV UNUSUAL_DEV
--
2.42.0
^ permalink raw reply related
* [RFC PATCH 5/6] usb-storage,uas,scsi: allow to pass through security commands (OPAL)
From: Milan Broz @ 2023-10-06 12:54 UTC (permalink / raw)
To: linux-usb
Cc: usb-storage, linux-scsi, linux-block, stern, oneukum,
jonathan.derrick, Milan Broz
In-Reply-To: <20231006125445.122380-1-gmazyland@gmail.com>
This patch enables pass-through OPAL command for USB-attached storage
(which does not support UAS or SCSI security commands).
All common USB/SATA or USB/NVMe adapters I tested need this patch.
USB mass storage devices that do not support SECURITY IN/OUT SCSI
commands can support ATA12 pass-thru command.
Internal kernel implementation for OPAL interface currently supports
only SCSI SECURITY IN/OUT wrapper.
USB mass storage also turns off SCSI command enumeration, so SECURITY
IN/OUT in the SCSI layer will be disabled.
Note: sedutils and some other OPAL tools already use ATA-12 pass-thru.
This patch
- enables SCSI security flag for USB mass storage and UAS device by default.
- adds an optional wrapper to the SCSI layer for the ATA-12 pass-thru
command as an alternative if SECURITY IN/OUT is unavailable.
In short, the patch runs these steps:
1) USB drives (mass-storage, UAS) enables security driver flag by default
if not disabled by quirk
2) SCSI device enumerates SECURITY IN/OUT support. If detected,
SECURITY ON/OUT wrapper is used (as in the current code).
If not, new ATA12 pass-thru wrapper is used instead.
3) SED OPAL code tries OPAL discovery command for device. If it receives
correct reply, OPAL is enabled for the device.
With the changes above, the TCG OPAL support works with USB adapters
that support the ATA-12 command. As kernel OPAL code runs discover
commands on initialization, on devices that do not support pass-through,
OPAL remains disabled.
The patch also adds a quirk flag to disable security commands for particular
devices if firmware is buggy.
Signed-off-by: Milan Broz <gmazyland@gmail.com>
---
drivers/scsi/sd.c | 33 +++++++++++++++++++++++++++++++--
drivers/usb/storage/scsiglue.c | 4 ++++
drivers/usb/storage/uas.c | 5 +++++
drivers/usb/storage/usb.c | 5 ++++-
include/linux/usb_usual.h | 2 ++
5 files changed, 46 insertions(+), 3 deletions(-)
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 83b6a3f3863b..3782556cb461 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -686,6 +686,32 @@ static int sd_sec_submit(void *data, u16 spsp, u8 secp, void *buffer,
&exec_args);
return ret <= 0 ? ret : -EIO;
}
+
+static int sd_ata12_submit(void *data, u16 spsp, u8 secp, void *buffer,
+ size_t len, bool send)
+{
+ struct scsi_disk *sdkp = data;
+ struct scsi_device *sdev = sdkp->device;
+ u8 cdb[12] = { 0, };
+ const struct scsi_exec_args exec_args = {
+ .req_flags = BLK_MQ_REQ_PM,
+ };
+ int ret;
+
+ cdb[0] = ATA_12;
+ cdb[1] = (send ? 5 /* ATA_PROTOCOL_PIO_DATA_IN */ : 4 /* ATA_PROTOCOL_PIO_DATA_OUT */) << 1;
+ cdb[2] = 2 /* t_length */ | (1 << 2) /* byt_blok */ | ((send ? 0 : 1) << 3) /* t_dir */;
+ cdb[3] = secp;
+ put_unaligned_le16(len / 512, &cdb[4]);
+ put_unaligned_le16(spsp, &cdb[6]);
+ cdb[9] = send ? 0x5e /* ATA_CMD_TRUSTED_SND */: 0x5c /* ATA_CMD_TRUSTED_RCV */;
+
+ ret = scsi_execute_cmd(sdev, cdb, send ? REQ_OP_DRV_OUT : REQ_OP_DRV_IN,
+ buffer, len, SD_TIMEOUT, sdkp->max_retries,
+ &exec_args);
+ return ret <= 0 ? ret : -EIO;
+}
+
#endif /* CONFIG_BLK_SED_OPAL */
/*
@@ -3699,8 +3725,11 @@ static int sd_probe(struct device *dev)
goto out;
}
- if (sdkp->security) {
- sdkp->opal_dev = init_opal_dev(sdkp, &sd_sec_submit);
+ if (sdp->security_supported) {
+ if (sdkp->security)
+ sdkp->opal_dev = init_opal_dev(sdkp, &sd_sec_submit);
+ else
+ sdkp->opal_dev = init_opal_dev(sdkp, &sd_ata12_submit);
if (sdkp->opal_dev)
sd_printk(KERN_NOTICE, sdkp, "supports TCG Opal\n");
}
diff --git a/drivers/usb/storage/scsiglue.c b/drivers/usb/storage/scsiglue.c
index c54e9805da53..ef93813a2049 100644
--- a/drivers/usb/storage/scsiglue.c
+++ b/drivers/usb/storage/scsiglue.c
@@ -209,6 +209,10 @@ static int slave_configure(struct scsi_device *sdev)
/* Do not attempt to use WRITE SAME */
sdev->no_write_same = 1;
+ /* Allow security commands (OPAL) passthrough */
+ if (!(us->fflags & US_FL_IGNORE_OPAL))
+ sdev->security_supported = 1;
+
/*
* Some disks return the total number of blocks in response
* to READ CAPACITY rather than the highest block number.
diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index f6e293daabf4..9dfe8ea20134 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -865,6 +865,11 @@ static int uas_slave_configure(struct scsi_device *sdev)
/* Some disks cannot handle WRITE_SAME */
if (devinfo->flags & US_FL_NO_SAME)
sdev->no_write_same = 1;
+
+ /* Allow security commands (OPAL) passthrough */
+ if (!(devinfo->flags & US_FL_IGNORE_OPAL))
+ sdev->security_supported = 1;
+
/*
* Some disks return the total number of blocks in response
* to READ CAPACITY rather than the highest block number.
diff --git a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c
index f3a53c3eeb45..04211ac803e4 100644
--- a/drivers/usb/storage/usb.c
+++ b/drivers/usb/storage/usb.c
@@ -478,7 +478,7 @@ void usb_stor_adjust_quirks(struct usb_device *udev, u64 *fflags)
US_FL_INITIAL_READ10 | US_FL_WRITE_CACHE |
US_FL_NO_ATA_1X | US_FL_NO_REPORT_OPCODES |
US_FL_MAX_SECTORS_240 | US_FL_NO_REPORT_LUNS |
- US_FL_ALWAYS_SYNC);
+ US_FL_ALWAYS_SYNC | US_FL_IGNORE_OPAL);
p = quirks;
while (*p) {
@@ -567,6 +567,9 @@ void usb_stor_adjust_quirks(struct usb_device *udev, u64 *fflags)
case 'y':
f |= US_FL_ALWAYS_SYNC;
break;
+ case 'z':
+ f |= US_FL_IGNORE_OPAL;
+ break;
/* Ignore unrecognized flag characters */
}
}
diff --git a/include/linux/usb_usual.h b/include/linux/usb_usual.h
index 712363c7a2e8..0181c94d7d91 100644
--- a/include/linux/usb_usual.h
+++ b/include/linux/usb_usual.h
@@ -88,6 +88,8 @@
/* Cannot handle WRITE_SAME */ \
US_FLAG(SENSE_AFTER_SYNC, 0x80000000) \
/* Do REQUEST_SENSE after SYNCHRONIZE_CACHE */ \
+ US_FLAG(IGNORE_OPAL, 0x100000000) \
+ /* Security commands (OPAL) are broken */ \
#define US_FLAG(name, value) US_FL_##name = value ,
enum { US_DO_ALL_FLAGS };
--
2.42.0
^ permalink raw reply related
* [RFC PATCH 4/6] usb-storage,uas: use host helper to generate driver info
From: Milan Broz @ 2023-10-06 12:54 UTC (permalink / raw)
To: linux-usb
Cc: usb-storage, linux-scsi, linux-block, stern, oneukum,
jonathan.derrick, Milan Broz
In-Reply-To: <20231006125445.122380-1-gmazyland@gmail.com>
The USB mass storage quirks flags are stored in driver_info,
a 32-bit integer (unsigned long on 32-bit platforms).
As this attribute cannot be enlarged, we need to use some form
of translation of 64-bit quirk bits.
This problem was discussed on USB list
https://lore.kernel.org/linux-usb/f9e8acb5-32d5-4a30-859f-d4336a86b31a@gmail.com/
The initial solution to use static array extensively increased the size
of the kernel module, so I decided to try the second suggested solution:
generate a table by host-compiled program and use bit 31 to indicate
that the value is index and not actual value.
This patch adds a host-compiled program that processes unusual_devs.h
(and unusual_uas.h) and generates files unusual-flags.c and unusual-flags-uas.c
(for pre-processed USB device table with 32 bit device info) and unusual-flags.h
with function to translate flags to 64-bits from device-info.
The separate generated header file is needed as storage and UAS drivers headers
are tightly bound together and any ohter solution seems to be more pervasive.
Translation function is used only in usb-storage and uas modules; all other
USB storage modules store flags directly, using only 32-bit integers.
This translation is unnecessary for a 64-bit system, but I keep it
in place for simplicity.
(Also, I did not find a reliable way a host-compiled program can detect
that the target platform has 32-bit unsigned long (usual macros do not
work here!).
Signed-off-by: Milan Broz <gmazyland@gmail.com>
---
drivers/usb/storage/Makefile | 25 ++++
drivers/usb/storage/mkflags.c | 212 +++++++++++++++++++++++++++++
drivers/usb/storage/uas-detect.h | 2 +-
drivers/usb/storage/uas.c | 17 +--
drivers/usb/storage/usb.c | 7 +-
drivers/usb/storage/usual-tables.c | 23 +---
6 files changed, 248 insertions(+), 38 deletions(-)
create mode 100644 drivers/usb/storage/mkflags.c
diff --git a/drivers/usb/storage/Makefile b/drivers/usb/storage/Makefile
index 46635fa4a340..1eacdbb387cd 100644
--- a/drivers/usb/storage/Makefile
+++ b/drivers/usb/storage/Makefile
@@ -45,3 +45,28 @@ ums-realtek-y := realtek_cr.o
ums-sddr09-y := sddr09.o
ums-sddr55-y := sddr55.o
ums-usbat-y := shuttle_usbat.o
+
+$(obj)/usb.o: $(obj)/unusual-flags.h
+$(obj)/usual-tables.o: $(obj)/unusual-flags.c
+$(obj)/uas.o: $(obj)/unusual-flags.h $(obj)/unusual-flags-uas.c
+clean-files := unusual-flags.h unusual-flags.c unusual-flags-uas.c
+HOSTCFLAGS_mkflags.o := -I $(srctree)/include/
+hostprogs += mkflags
+
+quiet_cmd_mkflag_flags = FLAGS $@
+ cmd_mkflag_flags = $(obj)/mkflags flags > $@
+
+quiet_cmd_mkflag_storage = FLAGS $@
+ cmd_mkflag_storage = $(obj)/mkflags storage > $@
+
+quiet_cmd_mkflag_uas = FLAGS $@
+ cmd_mkflag_uas = $(obj)/mkflags uas > $@
+
+$(obj)/unusual-flags.h: $(obj)/mkflags FORCE
+ $(call if_changed,mkflag_flags)
+
+$(obj)/unusual-flags.c: $(obj)/mkflags FORCE
+ $(call if_changed,mkflag_storage)
+
+$(obj)/unusual-flags-uas.c: $(obj)/mkflags FORCE
+ $(call if_changed,mkflag_uas)
diff --git a/drivers/usb/storage/mkflags.c b/drivers/usb/storage/mkflags.c
new file mode 100644
index 000000000000..11aa6579e7e1
--- /dev/null
+++ b/drivers/usb/storage/mkflags.c
@@ -0,0 +1,212 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+#include <stdio.h>
+#include <string.h>
+
+/*
+ * Cannot use userspace <inttypes.h> as code below
+ * processes internal kernel headers
+ */
+#include <linux/types.h>
+
+/*
+ * Silence warning for definitions in headers we do not use
+ */
+struct usb_device_id {};
+struct usb_interface;
+
+#include <linux/usb_usual.h>
+
+struct svals {
+ unsigned int type;
+
+ /*interface */
+ uint8_t bDeviceSubClass;
+ uint8_t bDeviceProtocol;
+
+ /* device */
+ uint16_t idVendor;
+ uint16_t idProduct;
+ uint16_t bcdDevice_lo;
+ uint16_t bcdDevice_hi;
+
+ uint64_t flags;
+ unsigned int set;
+ unsigned int idx;
+};
+
+enum { TYPE_DEVICE_STORAGE, TYPE_DEVICE_UAS, TYPE_CLASS };
+enum { FLAGS_NOT_SET, FLAGS_SET, FLAGS_DUPLICATE };
+#define FLAGS_END (uint64_t)-1
+
+#define IS_ENABLED(x) 0
+
+static struct svals vals[] = {
+#define USUAL_DEV(useProto, useTrans) \
+{ TYPE_CLASS, useProto, useTrans, 0, 0, 0, 0, 0, FLAGS_NOT_SET, 0 }
+
+#define COMPLIANT_DEV UNUSUAL_DEV
+#define UNUSUAL_DEV(id_vendor, id_product, bcdDeviceMin, bcdDeviceMax, \
+ vendorName, productName, useProtocol, useTransport, \
+ initFunction, flags) \
+{ TYPE_DEVICE_STORAGE, 0, 0, id_vendor, id_product, bcdDeviceMin, bcdDeviceMax, flags, FLAGS_NOT_SET, 0 }
+
+#include "unusual_devs.h"
+
+/* UAS */
+#undef UNUSUAL_DEV
+#define UNUSUAL_DEV(id_vendor, id_product, bcdDeviceMin, bcdDeviceMax, \
+ vendorName, productName, useProtocol, useTransport, \
+ initFunction, flags) \
+{ TYPE_DEVICE_UAS, 0, 0, id_vendor, id_product, bcdDeviceMin, bcdDeviceMax, flags, FLAGS_NOT_SET, 0 }
+
+#include "unusual_uas.h"
+
+{ .flags = FLAGS_END }
+};
+#undef UNUSUAL_DEV
+#undef USUAL_DEV
+#undef COMPLIANT_DEV
+#undef IS_ENABLED
+
+#define HI32 (uint32_t)0x80000000
+
+static unsigned long get_device_info(uint64_t flags, unsigned int idx)
+{
+ if (flags < HI32)
+ return (unsigned long)flags;
+
+ /* Use index that will be processed in usb_stor_di2flags */
+ return HI32 + idx;
+}
+
+static void print_class(uint8_t bDeviceSubClass, uint8_t bDeviceProtocol)
+{
+ printf("\t{ .match_flags = USB_DEVICE_ID_MATCH_INT_INFO, ");
+ printf(".bInterfaceClass = USB_CLASS_MASS_STORAGE, ");
+ printf(".bInterfaceSubClass = 0x%x, .bInterfaceProtocol = 0x%x },\n",
+ bDeviceSubClass, bDeviceProtocol);
+}
+static void print_type(unsigned int type)
+{
+ for (int i = 0; vals[i].flags != FLAGS_END; i++) {
+ if (vals[i].type != type)
+ continue;
+
+ if (type == TYPE_DEVICE_STORAGE || type == TYPE_DEVICE_UAS) {
+ printf("\t{ .match_flags = USB_DEVICE_ID_MATCH_DEVICE_AND_VERSION, ");
+ printf(".idVendor = 0x%x, .idProduct =0x%x, "
+ ".bcdDevice_lo = 0x%x, .bcdDevice_hi = 0x%x, .driver_info = 0x%lx },\n",
+ vals[i].idVendor, vals[i].idProduct,
+ vals[i].bcdDevice_lo, vals[i].bcdDevice_hi,
+ get_device_info(vals[i].flags, vals[i].idx));
+ } else if (type == TYPE_CLASS)
+ print_class(vals[i].bDeviceSubClass, vals[i].bDeviceProtocol);
+ }
+}
+
+static void print_usb_flags(void)
+{
+ int i;
+
+ printf("#include <linux/types.h>\n\n");
+
+ /* usb_stor_di2flags */
+ printf("static u64 usb_stor_di2flags(unsigned long idx)\n{\n");
+ printf("\tu64 flags = 0;\n\n");
+ printf("\tif (idx < 0x%x) \n\t\treturn idx;\n\n", HI32);
+ printf("\tswitch(idx - 0x%x) {\n", HI32);
+ for (i = 0; vals[i].flags != FLAGS_END; i++) {
+ if (vals[i].set == FLAGS_SET)
+ printf("\tcase %u: flags = 0x%llx; break;\n", vals[i].idx, vals[i].flags);
+ }
+ printf("\t}\n\n");
+ printf("\treturn flags;\n");
+ printf("}\n");
+}
+
+static void print_usb_storage(void)
+{
+ printf("#include <linux/usb.h>\n\n");
+
+ /* usb_storage_usb_ids */
+ printf("const struct usb_device_id usb_storage_usb_ids[] = {\n");
+
+ /* USB storage devices */
+ print_type(TYPE_DEVICE_STORAGE);
+
+ /* UAS storage devices */
+ printf("#if IS_ENABLED(CONFIG_USB_UAS)\n");
+ print_type(TYPE_DEVICE_UAS);
+ printf("#endif\n");
+
+ /* transport subclasses */
+ print_type(TYPE_CLASS);
+
+ printf("\t{ }\t\t/* Terminating entry */\n};\n");
+ printf("MODULE_DEVICE_TABLE(usb, usb_storage_usb_ids);\n");
+}
+
+static void print_usb_uas(void)
+{
+ printf("#include <linux/usb.h>\n\n");
+
+ /* uas_usb_ids */
+ printf("const struct usb_device_id uas_usb_ids[] = {\n");
+
+ /* UAS storage devices */
+ print_type(TYPE_DEVICE_UAS);
+
+ /* transport subclasses */
+ print_class(USB_SC_SCSI, USB_PR_BULK);
+ print_class(USB_SC_SCSI, USB_PR_UAS);
+
+ printf("\t{ }\t\t/* Terminating entry */\n};\n");
+ printf("MODULE_DEVICE_TABLE(usb, uas_usb_ids);\n");
+}
+
+int main(int argc, char *argv[])
+{
+ int i, j, idx = 0, idx_old, skip = 0;
+
+ if (argc != 2 || (strcmp(argv[1], "flags") &&
+ strcmp(argv[1], "storage") && strcmp(argv[1], "uas"))) {
+ printf("Please specify type: storage, uas or flags.\n");
+ return 1;
+ }
+
+ for (i = 0; vals[i].flags != FLAGS_END; i++) {
+ if (vals[i].type == TYPE_CLASS)
+ continue;
+ skip = 0;
+ if (vals[i].flags >= HI32) {
+ for (j = 0; j < i; j++) {
+ if (vals[j].flags == vals[i].flags &&
+ vals[j].set == FLAGS_SET) {
+ skip = 1;
+ idx_old = vals[j].idx;
+ break;
+ }
+ }
+ if (skip) {
+ vals[i].idx = idx_old;
+ vals[i].set = FLAGS_DUPLICATE;
+ } else {
+ vals[i].idx = idx;
+ vals[i].set = FLAGS_SET;
+ idx++;
+ }
+ }
+ }
+
+ if (!strcmp(argv[1], "flags"))
+ print_usb_flags();
+ else if (!strcmp(argv[1], "storage"))
+ print_usb_storage();
+ else if (!strcmp(argv[1], "uas"))
+ print_usb_uas();
+ else
+ return 1;
+
+ return 0;
+}
diff --git a/drivers/usb/storage/uas-detect.h b/drivers/usb/storage/uas-detect.h
index 4d3b49e5b87a..701621aaa9e7 100644
--- a/drivers/usb/storage/uas-detect.h
+++ b/drivers/usb/storage/uas-detect.h
@@ -59,7 +59,7 @@ static int uas_use_uas_driver(struct usb_interface *intf,
struct usb_host_endpoint *eps[4] = { };
struct usb_device *udev = interface_to_usbdev(intf);
struct usb_hcd *hcd = bus_to_hcd(udev->bus);
- u64 flags = id->driver_info;
+ u64 flags = usb_stor_di2flags(id->driver_info);
struct usb_host_interface *alt;
int r;
diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index 696bb0b23599..f6e293daabf4 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -26,6 +26,7 @@
#include <scsi/scsi_host.h>
#include <scsi/scsi_tcq.h>
+#include "unusual-flags.h"
#include "uas-detect.h"
#include "scsiglue.h"
@@ -909,21 +910,7 @@ static const struct scsi_host_template uas_host_template = {
.cmd_size = sizeof(struct uas_cmd_info),
};
-#define UNUSUAL_DEV(id_vendor, id_product, bcdDeviceMin, bcdDeviceMax, \
- vendorName, productName, useProtocol, useTransport, \
- initFunction, flags) \
-{ USB_DEVICE_VER(id_vendor, id_product, bcdDeviceMin, bcdDeviceMax), \
- .driver_info = (flags) }
-
-static struct usb_device_id uas_usb_ids[] = {
-# include "unusual_uas.h"
- { USB_INTERFACE_INFO(USB_CLASS_MASS_STORAGE, USB_SC_SCSI, USB_PR_BULK) },
- { USB_INTERFACE_INFO(USB_CLASS_MASS_STORAGE, USB_SC_SCSI, USB_PR_UAS) },
- { }
-};
-MODULE_DEVICE_TABLE(usb, uas_usb_ids);
-
-#undef UNUSUAL_DEV
+#include "unusual-flags-uas.c"
static int uas_switch_interface(struct usb_device *udev,
struct usb_interface *intf)
diff --git a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c
index 72b48b94aa5f..f3a53c3eeb45 100644
--- a/drivers/usb/storage/usb.c
+++ b/drivers/usb/storage/usb.c
@@ -56,6 +56,8 @@
#include "sierra_ms.h"
#include "option_ms.h"
+#include "unusual-flags.h"
+
#if IS_ENABLED(CONFIG_USB_UAS)
#include "uas-detect.h"
#endif
@@ -589,7 +591,10 @@ static int get_device_info(struct us_data *us, const struct usb_device_id *id,
us->protocol = (unusual_dev->useTransport == USB_PR_DEVICE) ?
idesc->bInterfaceProtocol :
unusual_dev->useTransport;
- us->fflags = id->driver_info;
+ if (fflags_use_index)
+ us->fflags = usb_stor_di2flags(id->driver_info);
+ else
+ us->fflags = id->driver_info;
usb_stor_adjust_quirks(us->pusb_dev, &us->fflags);
diff --git a/drivers/usb/storage/usual-tables.c b/drivers/usb/storage/usual-tables.c
index a26029e43dfd..c5871c1c3915 100644
--- a/drivers/usb/storage/usual-tables.c
+++ b/drivers/usb/storage/usual-tables.c
@@ -13,28 +13,9 @@
/*
- * The table of devices
+ * The table of devices is pre-generated in unusual-flags.c
*/
-#define UNUSUAL_DEV(id_vendor, id_product, bcdDeviceMin, bcdDeviceMax, \
- vendorName, productName, useProtocol, useTransport, \
- initFunction, flags) \
-{ USB_DEVICE_VER(id_vendor, id_product, bcdDeviceMin, bcdDeviceMax), \
- .driver_info = (kernel_ulong_t)(flags) }
-
-#define COMPLIANT_DEV UNUSUAL_DEV
-
-#define USUAL_DEV(useProto, useTrans) \
-{ USB_INTERFACE_INFO(USB_CLASS_MASS_STORAGE, useProto, useTrans) }
-
-const struct usb_device_id usb_storage_usb_ids[] = {
-# include "unusual_devs.h"
- { } /* Terminating entry */
-};
-MODULE_DEVICE_TABLE(usb, usb_storage_usb_ids);
-
-#undef UNUSUAL_DEV
-#undef COMPLIANT_DEV
-#undef USUAL_DEV
+#include "unusual-flags.c"
/*
* The table of devices to ignore
--
2.42.0
^ permalink raw reply related
* [RFC PATCH 3/6] usb-storage: use fflags index only in usb-storage driver
From: Milan Broz @ 2023-10-06 12:54 UTC (permalink / raw)
To: linux-usb
Cc: usb-storage, linux-scsi, linux-block, stern, oneukum,
jonathan.derrick, Milan Broz
In-Reply-To: <20231006125445.122380-1-gmazyland@gmail.com>
This patch adds a parameter to use driver_info translation function
(which will be defined in the following patch).
Only USB storage driver will use it, as other drivers do not need
more than 32-bit quirk flags.
Signed-off-by: Milan Broz <gmazyland@gmail.com>
---
drivers/usb/storage/alauda.c | 2 +-
drivers/usb/storage/cypress_atacb.c | 2 +-
drivers/usb/storage/datafab.c | 2 +-
drivers/usb/storage/ene_ub6250.c | 2 +-
drivers/usb/storage/freecom.c | 2 +-
drivers/usb/storage/isd200.c | 2 +-
drivers/usb/storage/jumpshot.c | 2 +-
drivers/usb/storage/karma.c | 2 +-
drivers/usb/storage/onetouch.c | 2 +-
drivers/usb/storage/realtek_cr.c | 2 +-
drivers/usb/storage/sddr09.c | 2 +-
drivers/usb/storage/sddr55.c | 2 +-
drivers/usb/storage/shuttle_usbat.c | 2 +-
drivers/usb/storage/usb.c | 10 ++++++----
drivers/usb/storage/usb.h | 3 ++-
15 files changed, 21 insertions(+), 18 deletions(-)
diff --git a/drivers/usb/storage/alauda.c b/drivers/usb/storage/alauda.c
index 115f05a6201a..74e293981ab1 100644
--- a/drivers/usb/storage/alauda.c
+++ b/drivers/usb/storage/alauda.c
@@ -1241,7 +1241,7 @@ static int alauda_probe(struct usb_interface *intf,
result = usb_stor_probe1(&us, intf, id,
(id - alauda_usb_ids) + alauda_unusual_dev_list,
- &alauda_host_template);
+ &alauda_host_template, 0);
if (result)
return result;
diff --git a/drivers/usb/storage/cypress_atacb.c b/drivers/usb/storage/cypress_atacb.c
index 98b3ec352a13..2fc939f709b0 100644
--- a/drivers/usb/storage/cypress_atacb.c
+++ b/drivers/usb/storage/cypress_atacb.c
@@ -246,7 +246,7 @@ static int cypress_probe(struct usb_interface *intf,
result = usb_stor_probe1(&us, intf, id,
(id - cypress_usb_ids) + cypress_unusual_dev_list,
- &cypress_host_template);
+ &cypress_host_template, 0);
if (result)
return result;
diff --git a/drivers/usb/storage/datafab.c b/drivers/usb/storage/datafab.c
index bcc4a2fad863..fad9eca3cad9 100644
--- a/drivers/usb/storage/datafab.c
+++ b/drivers/usb/storage/datafab.c
@@ -727,7 +727,7 @@ static int datafab_probe(struct usb_interface *intf,
result = usb_stor_probe1(&us, intf, id,
(id - datafab_usb_ids) + datafab_unusual_dev_list,
- &datafab_host_template);
+ &datafab_host_template, 0);
if (result)
return result;
diff --git a/drivers/usb/storage/ene_ub6250.c b/drivers/usb/storage/ene_ub6250.c
index 97c66c0d91f4..6985d3419b3c 100644
--- a/drivers/usb/storage/ene_ub6250.c
+++ b/drivers/usb/storage/ene_ub6250.c
@@ -2331,7 +2331,7 @@ static int ene_ub6250_probe(struct usb_interface *intf,
result = usb_stor_probe1(&us, intf, id,
(id - ene_ub6250_usb_ids) + ene_ub6250_unusual_dev_list,
- &ene_ub6250_host_template);
+ &ene_ub6250_host_template, 0);
if (result)
return result;
diff --git a/drivers/usb/storage/freecom.c b/drivers/usb/storage/freecom.c
index 2b098b55c4cb..6d971bd711d8 100644
--- a/drivers/usb/storage/freecom.c
+++ b/drivers/usb/storage/freecom.c
@@ -548,7 +548,7 @@ static int freecom_probe(struct usb_interface *intf,
result = usb_stor_probe1(&us, intf, id,
(id - freecom_usb_ids) + freecom_unusual_dev_list,
- &freecom_host_template);
+ &freecom_host_template, 0);
if (result)
return result;
diff --git a/drivers/usb/storage/isd200.c b/drivers/usb/storage/isd200.c
index 4e0eef1440b7..ecdc494756a2 100644
--- a/drivers/usb/storage/isd200.c
+++ b/drivers/usb/storage/isd200.c
@@ -1545,7 +1545,7 @@ static int isd200_probe(struct usb_interface *intf,
result = usb_stor_probe1(&us, intf, id,
(id - isd200_usb_ids) + isd200_unusual_dev_list,
- &isd200_host_template);
+ &isd200_host_template, 0);
if (result)
return result;
diff --git a/drivers/usb/storage/jumpshot.c b/drivers/usb/storage/jumpshot.c
index 229bf0c1afc9..1ade1e45c81d 100644
--- a/drivers/usb/storage/jumpshot.c
+++ b/drivers/usb/storage/jumpshot.c
@@ -653,7 +653,7 @@ static int jumpshot_probe(struct usb_interface *intf,
result = usb_stor_probe1(&us, intf, id,
(id - jumpshot_usb_ids) + jumpshot_unusual_dev_list,
- &jumpshot_host_template);
+ &jumpshot_host_template, 0);
if (result)
return result;
diff --git a/drivers/usb/storage/karma.c b/drivers/usb/storage/karma.c
index 38ddfedef629..60868be0e38c 100644
--- a/drivers/usb/storage/karma.c
+++ b/drivers/usb/storage/karma.c
@@ -205,7 +205,7 @@ static int karma_probe(struct usb_interface *intf,
result = usb_stor_probe1(&us, intf, id,
(id - karma_usb_ids) + karma_unusual_dev_list,
- &karma_host_template);
+ &karma_host_template, 0);
if (result)
return result;
diff --git a/drivers/usb/storage/onetouch.c b/drivers/usb/storage/onetouch.c
index 01f3c2779ccf..fe34f20cce03 100644
--- a/drivers/usb/storage/onetouch.c
+++ b/drivers/usb/storage/onetouch.c
@@ -280,7 +280,7 @@ static int onetouch_probe(struct usb_interface *intf,
result = usb_stor_probe1(&us, intf, id,
(id - onetouch_usb_ids) + onetouch_unusual_dev_list,
- &onetouch_host_template);
+ &onetouch_host_template, 0);
if (result)
return result;
diff --git a/drivers/usb/storage/realtek_cr.c b/drivers/usb/storage/realtek_cr.c
index 0c423916d7bf..892b26714b5f 100644
--- a/drivers/usb/storage/realtek_cr.c
+++ b/drivers/usb/storage/realtek_cr.c
@@ -1041,7 +1041,7 @@ static int realtek_cr_probe(struct usb_interface *intf,
result = usb_stor_probe1(&us, intf, id,
(id - realtek_cr_ids) +
realtek_cr_unusual_dev_list,
- &realtek_cr_host_template);
+ &realtek_cr_host_template, 0);
if (result)
return result;
diff --git a/drivers/usb/storage/sddr09.c b/drivers/usb/storage/sddr09.c
index 51bcd4a43690..107eeb7fda04 100644
--- a/drivers/usb/storage/sddr09.c
+++ b/drivers/usb/storage/sddr09.c
@@ -1753,7 +1753,7 @@ static int sddr09_probe(struct usb_interface *intf,
result = usb_stor_probe1(&us, intf, id,
(id - sddr09_usb_ids) + sddr09_unusual_dev_list,
- &sddr09_host_template);
+ &sddr09_host_template, 0);
if (result)
return result;
diff --git a/drivers/usb/storage/sddr55.c b/drivers/usb/storage/sddr55.c
index 15dc25801cdc..c64b72de453f 100644
--- a/drivers/usb/storage/sddr55.c
+++ b/drivers/usb/storage/sddr55.c
@@ -985,7 +985,7 @@ static int sddr55_probe(struct usb_interface *intf,
result = usb_stor_probe1(&us, intf, id,
(id - sddr55_usb_ids) + sddr55_unusual_dev_list,
- &sddr55_host_template);
+ &sddr55_host_template, 0);
if (result)
return result;
diff --git a/drivers/usb/storage/shuttle_usbat.c b/drivers/usb/storage/shuttle_usbat.c
index f0d0ca37163d..3ac82f49401c 100644
--- a/drivers/usb/storage/shuttle_usbat.c
+++ b/drivers/usb/storage/shuttle_usbat.c
@@ -1838,7 +1838,7 @@ static int usbat_probe(struct usb_interface *intf,
result = usb_stor_probe1(&us, intf, id,
(id - usbat_usb_ids) + usbat_unusual_dev_list,
- &usbat_host_template);
+ &usbat_host_template, 0);
if (result)
return result;
diff --git a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c
index d1ad6a2509ab..72b48b94aa5f 100644
--- a/drivers/usb/storage/usb.c
+++ b/drivers/usb/storage/usb.c
@@ -574,7 +574,7 @@ EXPORT_SYMBOL_GPL(usb_stor_adjust_quirks);
/* Get the unusual_devs entries and the string descriptors */
static int get_device_info(struct us_data *us, const struct usb_device_id *id,
- const struct us_unusual_dev *unusual_dev)
+ const struct us_unusual_dev *unusual_dev, int fflags_use_index)
{
struct usb_device *dev = us->pusb_dev;
struct usb_interface_descriptor *idesc =
@@ -590,6 +590,7 @@ static int get_device_info(struct us_data *us, const struct usb_device_id *id,
idesc->bInterfaceProtocol :
unusual_dev->useTransport;
us->fflags = id->driver_info;
+
usb_stor_adjust_quirks(us->pusb_dev, &us->fflags);
if (us->fflags & US_FL_IGNORE_DEVICE) {
@@ -925,7 +926,8 @@ int usb_stor_probe1(struct us_data **pus,
struct usb_interface *intf,
const struct usb_device_id *id,
const struct us_unusual_dev *unusual_dev,
- const struct scsi_host_template *sht)
+ const struct scsi_host_template *sht,
+ int fflags_use_index)
{
struct Scsi_Host *host;
struct us_data *us;
@@ -962,7 +964,7 @@ int usb_stor_probe1(struct us_data **pus,
goto BadDevice;
/* Get the unusual_devs entries and the descriptors */
- result = get_device_info(us, id, unusual_dev);
+ result = get_device_info(us, id, unusual_dev, fflags_use_index);
if (result)
goto BadDevice;
@@ -1120,7 +1122,7 @@ static int storage_probe(struct usb_interface *intf,
}
result = usb_stor_probe1(&us, intf, id, unusual_dev,
- &usb_stor_host_template);
+ &usb_stor_host_template, 1);
if (result)
return result;
diff --git a/drivers/usb/storage/usb.h b/drivers/usb/storage/usb.h
index 97c6196d639b..975c47efcce7 100644
--- a/drivers/usb/storage/usb.h
+++ b/drivers/usb/storage/usb.h
@@ -187,7 +187,8 @@ extern int usb_stor_probe1(struct us_data **pus,
struct usb_interface *intf,
const struct usb_device_id *id,
const struct us_unusual_dev *unusual_dev,
- const struct scsi_host_template *sht);
+ const struct scsi_host_template *sht,
+ int fflags_use_index);
extern int usb_stor_probe2(struct us_data *us);
extern void usb_stor_disconnect(struct usb_interface *intf);
--
2.42.0
^ permalink raw reply related
* [RFC PATCH 6/6] usb-storage,uas: Disable security commands (OPAL) for RT9210 chip family
From: Milan Broz @ 2023-10-06 12:54 UTC (permalink / raw)
To: linux-usb
Cc: usb-storage, linux-scsi, linux-block, stern, oneukum,
jonathan.derrick, Milan Broz
In-Reply-To: <20231006125445.122380-1-gmazyland@gmail.com>
Realtek 9210 family (NVME to USB bridge) adapters always set
the write-protected bit for the whole drive if an OPAL locking range
is defined (even if the OPAL locking range just covers part of the disk).
The only way to recover is PSID reset and physical reconnection of the device.
This looks like a wrong implementation of OPAL standard (and I will try
to report it to Realtek as it happens for all firmware versions I have),
but for now, these adapters are unusable for OPAL.
Signed-off-by: Milan Broz <gmazyland@gmail.com>
---
drivers/usb/storage/unusual_devs.h | 11 +++++++++++
drivers/usb/storage/unusual_uas.h | 11 +++++++++++
2 files changed, 22 insertions(+)
diff --git a/drivers/usb/storage/unusual_devs.h b/drivers/usb/storage/unusual_devs.h
index 20dcbccb290b..b7c0df180e5d 100644
--- a/drivers/usb/storage/unusual_devs.h
+++ b/drivers/usb/storage/unusual_devs.h
@@ -1476,6 +1476,17 @@ UNUSUAL_DEV( 0x0bc2, 0x3332, 0x0000, 0x9999,
USB_SC_DEVICE, USB_PR_DEVICE, NULL,
US_FL_NO_WP_DETECT ),
+/*
+ * Realtek 9210 family set global write-protection flag
+ * for any OPAL locking range making device unusable
+ * Reported-by: Milan Broz <gmazyland@gmail.com>
+ */
+UNUSUAL_DEV( 0x0bda, 0x9210, 0x0000, 0xffff,
+ "Realtek",
+ "",
+ USB_SC_DEVICE, USB_PR_DEVICE, NULL,
+ US_FL_IGNORE_OPAL),
+
UNUSUAL_DEV( 0x0d49, 0x7310, 0x0000, 0x9999,
"Maxtor",
"USB to SATA",
diff --git a/drivers/usb/storage/unusual_uas.h b/drivers/usb/storage/unusual_uas.h
index 1f8c9b16a0fb..71ab824bfb32 100644
--- a/drivers/usb/storage/unusual_uas.h
+++ b/drivers/usb/storage/unusual_uas.h
@@ -185,3 +185,14 @@ UNUSUAL_DEV(0x4971, 0x8024, 0x0000, 0x9999,
"External HDD",
USB_SC_DEVICE, USB_PR_DEVICE, NULL,
US_FL_ALWAYS_SYNC),
+
+/*
+ * Realtek 9210 family set global write-protection flag
+ * for any OPAL locking range making device unusable
+ * Reported-by: Milan Broz <gmazyland@gmail.com>
+ */
+UNUSUAL_DEV(0x0bda, 0x9210, 0x0000, 0xffff,
+ "Realtek",
+ "",
+ USB_SC_DEVICE, USB_PR_DEVICE, NULL,
+ US_FL_IGNORE_OPAL),
--
2.42.0
^ permalink raw reply related
* RE: [PATCH v19 1/4] usb: Add support for Intel LJCA device
From: Wu, Wentong @ 2023-10-06 13:07 UTC (permalink / raw)
To: Greg KH
Cc: arnd@arndb.de, mka@chromium.org, oneukum@suse.com, lee@kernel.org,
wsa@kernel.org, kfting@nuvoton.com, broonie@kernel.org,
linus.walleij@linaro.org, hdegoede@redhat.com, maz@kernel.org,
brgl@bgdev.pl, linux-usb@vger.kernel.org,
linux-i2c@vger.kernel.org, linux-spi@vger.kernel.org,
linux-gpio@vger.kernel.org, andriy.shevchenko@linux.intel.com,
heikki.krogerus@linux.intel.com, andi.shyti@linux.intel.com,
sakari.ailus@linux.intel.com, bartosz.golaszewski@linaro.org,
Pandruvada, Srinivas, Wang, Zhifeng
In-Reply-To: <DM6PR11MB4316BCF3E25A54F7EB5E331C8DC4A@DM6PR11MB4316.namprd11.prod.outlook.com>
> From: Wu, Wentong
> > From: Greg KH <gregkh@linuxfoundation.org> On Fri, Sep 29, 2023 at
> > 11:31:21AM +0000, Wu, Wentong wrote:
> > > > From: Greg KH <gregkh@linuxfoundation.org> On Sun, Sep 17, 2023 at
> > > > 02:53:33AM +0800, Wentong Wu wrote:
> > > > > +static void ljca_handle_event(struct ljca_adapter *adap,
> > > > > + struct ljca_msg *header) {
> > > > > + struct ljca_client *client;
> > > > > +
> > > > > + list_for_each_entry(client, &adap->client_list, link) {
> > > > > + /*
> > > > > + * FIXME: currently only GPIO register event callback.
> > > > > + * firmware message structure should include id when
> > > > > + * multiple same type clients register event callback.
> > > > > + */
> > > >
> > > > When will this be fixed?
> > > >
> > > > If not now, why not?
> > >
> > > Actually this doesn't impact current functionality because only GPIO
> > > register event callback, but from coding perspective it should add
> > > the id in the message structure. This fix should be done by firmware
> > > first, but many products have already been running the firmware,
> > > it's not easy
> > to update the firmware.
> > >
> > > Can I just remove the 'FIXME' and leave the comment here?
> >
> > If you are never going to fix it, it does not need a FIXME, right? :)
>
> Thanks, I will remove 'FIXME' here.
>
> > > > You control both of these types, why aren't they the same type to start
> with?
> > > > Why the force cast?
> > >
> > > The len of header is defined by firmware, it should be u8 type. But
> > > the ex_buf_len is passed by API users, I don't want users to do the
> > > cast if their buffer is large than 256.
> >
> > Then fix the api to use a u8 as obviously you can not handle more data
> > then that for now.
>
> Ack, thanks
>
> > > > Do you really need a scoped guard when you can not fail out of the block?
> > >
> > > The scoped_guard is required by you with "Why not use the
> > > functionality in cleanup.h for this lock? Makes this function much
> > > simpler." If I understand correctly, so I use the scoped_guard where
> > > I can to
> > make things simpler.
> >
> > But that's not making anything simpler here, cleanup.h works well when
> > you have error paths that would be more complex without it. You do
> > not have that here at all now (maybe you did before?)
>
> I'll remove scoped guard
>
> >
> > > > Why do you have both a mutex and spinlock grabed?
> > >
> > > The mutex is to avoid command download concurrently
> > >
> > > The spinlock is to protect tx_buf and ex_buf, which may be accessed
> > > by tx and rx at the same time.
> >
> > Please document this somewhere.
>
> Ack, thanks. Below is the kernel doc for struct ljca_adapter where we have
> spinlock and mutex document.
>
> /**
> * struct ljca_adapter - represent a ljca adapter
> *
> * @intf: the usb interface for this ljca adapter
> * @usb_dev: the usb device for this ljca adapter
> * @dev: the specific device info of the usb interface
> * @rx_pipe: bulk in pipe for receive data from firmware
> * @tx_pipe: bulk out pipe for send data to firmware
> * @rx_urb: urb used for the bulk in pipe
> * @rx_buf: buffer used to receive command response and event
> * @rx_len: length of rx buffer
> * @ex_buf: external buffer to save command response
> * @ex_buf_len: length of external buffer
> * @actual_length: actual length of data copied to external buffer
> * @tx_buf: buffer used to download command to firmware
> * @tx_buf_len: length of tx buffer
> * @lock: spinlock to protect tx_buf and ex_buf
> * @cmd_completion: completion object as the command receives ack
> * @mutex: mutex to avoid command download concurrently
> * @client_list: client device list
> * @disconnect: usb disconnect ongoing or not
> * @reset_id: used to reset firmware
> */
> struct ljca_adapter {
> struct usb_interface *intf;
> struct usb_device *usb_dev;
> struct device *dev;
>
> unsigned int rx_pipe;
> unsigned int tx_pipe;
>
> struct urb *rx_urb;
> void *rx_buf;
> unsigned int rx_len;
>
> u8 *ex_buf;
> u8 ex_buf_len;
> u8 actual_length;
>
> void *tx_buf;
> u8 tx_buf_len;
>
> spinlock_t lock;
>
> struct completion cmd_completion;
> struct mutex mutex;
>
> struct list_head client_list;
>
> bool disconnect;
>
> u32 reset_id;
> };
>
> > > > Why are you not verifying that you sent what you wanted to send?
> > >
> > > As I said, the actual_length is the actual length of data copied to
> > > user buffer instead of the length of what we want to send.
> > >
> > > And even for verifying the length of what we want to send, the max
> > > length of the message is sizeof(struct ljca_msg) +
> > > LJCA_MAX_PACKET_SIZE which is less than the endpoint's max packet
> > > size, so I don't check the actual sent length in above usb_bulk_msg().
> >
> > But you need to.
>
> Ack, thanks.
>
> >
> > > > When you call this function, sometimes you check that the function
> > > > sent the proper amount of data, but in many places you do not, and
> > > > you assume that the full buffer was sent, which is not correct.
> > > > So please change _this_ function to check that you sent the proper
> > > > amount and then the caller logic will be much simpler and actually
> > > > work like you are using it in many places (some places you got it
> > > > right, some wrong, which is a HUGE indication that the API is
> > > > wrong because you wrote this code, and if you can't get it
> > > > right...)
> > >
> > > As I said, the return value of this function is the response data
> > > length instead of sent data length. And in this patch set, every
> > > caller has verified if the response length matched with the sent command.
> >
> > No, I found many users that did not do this.
>
> Ack, I will check more, thanks
>
> >Please make the api easy to use, right now it's not.
>
> I post the new ljca_send() here to try to address several comments.
>
> First it changes the type of buffer length to u8, second it checks the actual sent of
> length usb_bulk_msg(). It removes the scoped guard as well.
>
> And below gives an explanation of the parameters and return value.
>
> The parameters(type, cmd, obuf_len) are used to construct message header, obuf
> is used for message data. And ibuf and ibuf_len are for response buffer and buffer
> length passed by API users. ack indicates if the command need an ack from
> firmware, timeout is timeout value to wait command ack.
>
> And the return value is the response copied to response buffer passed by API
> users, normally the users know how large buffer they have to pass to this API, but
> from coding perspective we should check the passed response buffer
> length(ibuf_len) and return the actual copied data length to the buffer.
>
> Hope that helps, thanks
>
> static int ljca_send(struct ljca_adapter *adap, u8 type, u8 cmd,
> const u8 *obuf, u8 obuf_len, u8 *ibuf, u8 ibuf_len,
> bool ack, unsigned long timeout) {
> unsigned int msg_len = sizeof(struct ljca_msg) + obuf_len;
> struct ljca_msg *header = adap->tx_buf;
> unsigned int transferred;
> unsigned long flags;
> int ret;
>
> if (adap->disconnect)
> return -ENODEV;
>
> if (msg_len > adap->tx_buf_len)
> return -EINVAL;
>
> mutex_lock(&adap->mutex);
>
> spin_lock_irqsave(&adap->lock, flags);
>
> header->type = type;
> header->cmd = cmd;
> header->len = obuf_len;
> if (obuf)
> memcpy(header->data, obuf, obuf_len);
>
> header->flags = LJCA_CMPL_FLAG | (ack ? LJCA_ACK_FLAG : 0);
>
> adap->ex_buf = ibuf;
> adap->ex_buf_len = ibuf_len;
> adap->actual_length = 0;
>
> spin_unlock_irqrestore(&adap->lock, flags);
>
> reinit_completion(&adap->cmd_completion);
>
> ret = usb_autopm_get_interface(adap->intf);
> if (ret < 0)
> goto out;
>
> ret = usb_bulk_msg(adap->usb_dev, adap->tx_pipe, header,
> msg_len, &transferred, LJCA_WRITE_TIMEOUT_MS);
>
> usb_autopm_put_interface(adap->intf);
>
> if (ret < 0)
> goto out;
> if (transferred != msg_len) {
> ret = -EIO;
> goto out;
> }
>
> if (ack) {
> ret = wait_for_completion_timeout(&adap->cmd_completion,
> timeout);
> if (!ret) {
> ret = -ETIMEDOUT;
> goto out;
> }
> }
> ret = adap->actual_length;
>
> out:
> spin_lock_irqsave(&adap->lock, flags);
> adap->ex_buf = NULL;
> adap->ex_buf_len = 0;
>
> memset(header, 0, sizeof(*header));
> spin_unlock_irqrestore(&adap->lock, flags);
>
> mutex_unlock(&adap->mutex);
>
> return ret;
> }
>
> > > > Are you sure this is a valid uid to use? If so, why? What
> > > > happens if this gets set to "0" for multiple ones? Don't
> > > > underestimate broken firmware tables, but also don't paper over
> > > > problems in ways that will be impossible to notice and can cause problems.
> > >
> > > This actually has been discussed in previous email as bellow:
> > >
> > > some DSDTs have only 1 ACPI companion for the 2 I2C controllers and
> > > these don't set a UID at all. On these models only the first I2C
> > > controller is used. So if a HID match has no UID use "0" for the HID.
> > > assigning the ACPI companion to the first I2C controller.
> > > An example device with this setup is the Dell Latitude 9420.
> >
> > Then document this really really well in the code itself, otherwise it looks
> broken.
>
> Ack, thanks, I post the new ljca_match_device_ids() here, hope that helps, thanks.
>
> static int ljca_match_device_ids(struct acpi_device *adev, void *data) {
> struct ljca_match_ids_walk_data *wd = data;
> const char *uid = acpi_device_uid(adev);
>
> if (acpi_match_device_ids(adev, wd->ids))
> return 0;
>
> if (!wd->uid)
> goto match;
>
> if (!uid)
> /*
> * Some DSDTs have only one ACPI companion for the two I2C
> * controllers and they don't set a UID at all (e.g. Dell
> * Latitude 9420). On these platforms only the first I2C
> * controller is used, so if a HID match has no UID we use
> * "0" as the UID and assign ACPI companion to the first
> * I2C controller.
> */
> uid = "0";
> else
> uid = strchr(uid, wd->uid[0]);
>
> if (!uid || strcmp(uid, wd->uid))
> return 0;
>
> match:
> wd->adev = adev;
>
> return 1;
> }
>
> > > This is to be compatible with old version FW which does not support
> > > full USB2xxx functions, so it just warn here as this is.
> >
> > Why do you have to support obsolete and broken firmware? Can't it be
> updated?
>
> make sense, and probably they have to update, thanks
>
> > You warn, yet return success? Again, that doesn't work well as you
> > never know if you need to unwind it or not.
> >
> > Either report an error and handle it, or don't, but what you have here
> > looks broken as-is.
>
> Ack, it should report error and handle it. Thanks, the function will be like below:
>
> static int ljca_enumerate_clients(struct ljca_adapter *adap) {
> struct ljca_client *client, *next;
> int ret;
>
> ret = ljca_reset_handshake(adap);
> if (ret)
> goto err_kill;
>
> ret = ljca_enumerate_gpio(adap);
> if (ret) {
> dev_err(adap->dev, "enumerate GPIO error\n");
> goto err_kill;
> }
>
> ret = ljca_enumerate_i2c(adap);
> if (ret) {
> dev_err(adap->dev, "enumerate I2C error\n");
> goto err_kill;
> }
>
> ret = ljca_enumerate_spi(adap);
> if (ret) {
> dev_err(adap->dev, "enumerate SPI error\n");
> goto err_kill;
> }
>
> return 0;
>
> err_kill:
> adap->disconnect = true;
>
> usb_kill_urb(adap->rx_urb);
>
> list_for_each_entry_safe_reverse(client, next, &adap->client_list, link) {
> auxiliary_device_delete(&client->auxdev);
> auxiliary_device_uninit(&client->auxdev);
>
> list_del_init(&client->link);
> kfree(client);
> }
>
> return ret;
> }
>
Hi Greg,
Could you please share your comment about this? If no more comment, I will
sent out next version patch set. Thanks
BR,
Wentong
>
> >
> > thanks,
> >
> > greg k-h
^ permalink raw reply
* [PATCH RFT v6 0/6] ARM: pxa: GPIO descriptor conversions
From: Duje Mihanović @ 2023-10-06 13:44 UTC (permalink / raw)
To: Daniel Mack, Haojian Zhuang, Robert Jarzmik, Russell King,
Alan Stern, Greg Kroah-Hartman, Linus Walleij,
Bartosz Golaszewski, Andy Shevchenko, Dmitry Torokhov, Mark Brown
Cc: linux-arm-kernel, linux-kernel, linux-usb, linux-gpio,
linux-input, linux-spi, Duje Mihanović, Bartosz Golaszewski
Hello,
Small series to convert some of the board files in the mach-pxa directory
to use the new GPIO descriptor interface.
Most notably, the am200epd, am300epd and Spitz matrix keypad among
others are not converted in this series.
Signed-off-by: Duje Mihanović <duje.mihanovic@skole.hr>
---
Changes in v6:
- Address maintainer comments:
- Use devm_gpiod_get_optional() in OHCI
- Use gpiod_get_array() in Spitz LEDs
- Update trailers
- Link to v5: https://lore.kernel.org/r/20231004-pxa-gpio-v5-0-d99ae6fceea8@skole.hr
Changes in v5:
- Address maintainer comments:
- Rename "reset generator" GPIO to "reset"
- Rename ads7846_wait_for_sync() to ads7846_wait_for_sync_gpio()
- Properly bail out when requesting USB host GPIO fails
- Use dev_err_probe() when requesting touchscreen sync GPIO fails
- Use static gpio_desc for gumstix bluetooth reset
- Pulse gumstix bluetooth reset line correctly (assert, then deassert)
- Fix style issue in ads7846_wait_for_sync_gpio()
- Update trailers
- Link to v4: https://lore.kernel.org/r/20231001-pxa-gpio-v4-0-0f3b975e6ed5@skole.hr
Changes in v4:
- Address maintainer comments:
- Move wait_for_sync() from spitz.c to driver
- Register LED platform device before getting its gpiod-s
- Add Linus' Reviewed-by
- Link to v3: https://lore.kernel.org/r/20230929-pxa-gpio-v3-0-af8d5e5d1f34@skole.hr
Changes in v3:
- Address maintainer comments:
- Use GPIO_LOOKUP_IDX for LEDs
- Drop unnecessary NULL assignments
- Don't give up on *all* SPI devices if hsync cannot be set up
- Add Linus' Acked-by
- Link to v2: https://lore.kernel.org/r/20230926-pxa-gpio-v2-0-984464d165dd@skole.hr
Changes in v2:
- Address maintainer comments:
- Change mentions of function to function()
- Drop cast in OHCI driver dev_warn() call
- Use %pe in OHCI and reset drivers
- Use GPIO _optional() API in OHCI driver
- Drop unnecessary not-null check in OHCI driver
- Use pr_err() instead of printk() in reset driver
- Rebase on v6.6-rc3
- Link to v1: https://lore.kernel.org/r/20230924-pxa-gpio-v1-0-2805b87d8894@skole.hr
---
Duje Mihanović (6):
ARM: pxa: Convert Spitz OHCI to GPIO descriptors
ARM: pxa: Convert Spitz LEDs to GPIO descriptors
ARM: pxa: Convert Spitz CF power control to GPIO descriptors
ARM: pxa: Convert reset driver to GPIO descriptors
ARM: pxa: Convert gumstix Bluetooth to GPIO descriptors
input: ads7846: Move wait_for_sync() logic to driver
arch/arm/mach-pxa/gumstix.c | 22 ++++++------
arch/arm/mach-pxa/reset.c | 39 +++++++-------------
arch/arm/mach-pxa/reset.h | 3 +-
arch/arm/mach-pxa/spitz.c | 71 +++++++++++++++++++++++++------------
drivers/input/touchscreen/ads7846.c | 22 ++++++++----
drivers/usb/host/ohci-pxa27x.c | 7 ++++
include/linux/spi/ads7846.h | 1 -
7 files changed, 96 insertions(+), 69 deletions(-)
---
base-commit: 8a749fd1a8720d4619c91c8b6e7528c0a355c0aa
change-id: 20230807-pxa-gpio-3ce25d574814
Best regards,
--
Duje Mihanović <duje.mihanovic@skole.hr>
^ permalink raw reply
* [PATCH RFT v6 2/6] ARM: pxa: Convert Spitz LEDs to GPIO descriptors
From: Duje Mihanović @ 2023-10-06 13:44 UTC (permalink / raw)
To: Daniel Mack, Haojian Zhuang, Robert Jarzmik, Russell King,
Alan Stern, Greg Kroah-Hartman, Linus Walleij,
Bartosz Golaszewski, Andy Shevchenko, Dmitry Torokhov, Mark Brown
Cc: linux-arm-kernel, linux-kernel, linux-usb, linux-gpio,
linux-input, linux-spi, Duje Mihanović
In-Reply-To: <20231006-pxa-gpio-v6-0-981b4910d599@skole.hr>
Sharp's Spitz board still uses the legacy GPIO interface for configuring
its two onboard LEDs.
Convert them to use the GPIO descriptor interface.
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Duje Mihanović <duje.mihanovic@skole.hr>
---
arch/arm/mach-pxa/spitz.c | 20 ++++++++++++++++++--
1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-pxa/spitz.c b/arch/arm/mach-pxa/spitz.c
index 535e2b2e997b..1fb4102ea39e 100644
--- a/arch/arm/mach-pxa/spitz.c
+++ b/arch/arm/mach-pxa/spitz.c
@@ -452,16 +452,25 @@ static inline void spitz_keys_init(void) {}
* LEDs
******************************************************************************/
#if defined(CONFIG_LEDS_GPIO) || defined(CONFIG_LEDS_GPIO_MODULE)
+static struct gpiod_lookup_table spitz_led_gpio_table = {
+ .dev_id = "leds-gpio",
+ .table = {
+ GPIO_LOOKUP_IDX("pxa-gpio", SPITZ_GPIO_LED_ORANGE, NULL, 0,
+ GPIO_ACTIVE_HIGH),
+ GPIO_LOOKUP_IDX("pxa-gpio", SPITZ_GPIO_LED_GREEN, NULL, 1,
+ GPIO_ACTIVE_HIGH),
+ { }
+ }
+};
+
static struct gpio_led spitz_gpio_leds[] = {
{
.name = "spitz:amber:charge",
.default_trigger = "sharpsl-charge",
- .gpio = SPITZ_GPIO_LED_ORANGE,
},
{
.name = "spitz:green:hddactivity",
.default_trigger = "disk-activity",
- .gpio = SPITZ_GPIO_LED_GREEN,
},
};
@@ -480,7 +489,14 @@ static struct platform_device spitz_led_device = {
static void __init spitz_leds_init(void)
{
+ struct gpio_descs *leds;
+
+ gpiod_add_lookup_table(&spitz_led_gpio_table);
platform_device_register(&spitz_led_device);
+ leds = gpiod_get_array_optional(&spitz_led_device.dev,
+ NULL, GPIOD_ASIS);
+ spitz_gpio_leds[0].gpiod = leds->desc[0];
+ spitz_gpio_leds[1].gpiod = leds->desc[1];
}
#else
static inline void spitz_leds_init(void) {}
--
2.42.0
^ permalink raw reply related
* [PATCH RFT v6 1/6] ARM: pxa: Convert Spitz OHCI to GPIO descriptors
From: Duje Mihanović @ 2023-10-06 13:44 UTC (permalink / raw)
To: Daniel Mack, Haojian Zhuang, Robert Jarzmik, Russell King,
Alan Stern, Greg Kroah-Hartman, Linus Walleij,
Bartosz Golaszewski, Andy Shevchenko, Dmitry Torokhov, Mark Brown
Cc: linux-arm-kernel, linux-kernel, linux-usb, linux-gpio,
linux-input, linux-spi, Duje Mihanović
In-Reply-To: <20231006-pxa-gpio-v6-0-981b4910d599@skole.hr>
Sharp's Spitz board still uses the legacy GPIO interface for controlling
a GPIO pin related to the USB host controller.
Convert this function to use the new GPIO descriptor interface.
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Duje Mihanović <duje.mihanovic@skole.hr>
---
arch/arm/mach-pxa/spitz.c | 13 ++++++-------
drivers/usb/host/ohci-pxa27x.c | 7 +++++++
2 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/arch/arm/mach-pxa/spitz.c b/arch/arm/mach-pxa/spitz.c
index cc691b199429..535e2b2e997b 100644
--- a/arch/arm/mach-pxa/spitz.c
+++ b/arch/arm/mach-pxa/spitz.c
@@ -649,23 +649,22 @@ static inline void spitz_mmc_init(void) {}
* USB Host
******************************************************************************/
#if defined(CONFIG_USB_OHCI_HCD) || defined(CONFIG_USB_OHCI_HCD_MODULE)
+GPIO_LOOKUP_SINGLE(spitz_usb_host_gpio_table, "pxa27x-ohci", "gpio-pxa",
+ SPITZ_GPIO_USB_HOST, "usb-host", GPIO_ACTIVE_LOW);
+
static int spitz_ohci_init(struct device *dev)
{
- int err;
-
- err = gpio_request(SPITZ_GPIO_USB_HOST, "USB_HOST");
- if (err)
- return err;
+ gpiod_add_lookup_table(&spitz_usb_host_gpio_table);
/* Only Port 2 is connected, setup USB Port 2 Output Control Register */
UP2OCR = UP2OCR_HXS | UP2OCR_HXOE | UP2OCR_DPPDE | UP2OCR_DMPDE;
- return gpio_direction_output(SPITZ_GPIO_USB_HOST, 1);
+ return 0;
}
static void spitz_ohci_exit(struct device *dev)
{
- gpio_free(SPITZ_GPIO_USB_HOST);
+ gpiod_remove_lookup_table(&spitz_usb_host_gpio_table);
}
static struct pxaohci_platform_data spitz_ohci_platform_data = {
diff --git a/drivers/usb/host/ohci-pxa27x.c b/drivers/usb/host/ohci-pxa27x.c
index 357d9aee38a3..7f04421c80d6 100644
--- a/drivers/usb/host/ohci-pxa27x.c
+++ b/drivers/usb/host/ohci-pxa27x.c
@@ -121,6 +121,7 @@ struct pxa27x_ohci {
void __iomem *mmio_base;
struct regulator *vbus[3];
bool vbus_enabled[3];
+ struct gpio_desc *usb_host;
};
#define to_pxa27x_ohci(hcd) (struct pxa27x_ohci *)(hcd_to_ohci(hcd)->priv)
@@ -447,6 +448,10 @@ static int ohci_hcd_pxa27x_probe(struct platform_device *pdev)
pxa_ohci = to_pxa27x_ohci(hcd);
pxa_ohci->clk = usb_clk;
pxa_ohci->mmio_base = (void __iomem *)hcd->regs;
+ pxa_ohci->usb_host = devm_gpiod_get_optional(&pdev->dev, "usb-host", GPIOD_OUT_LOW);
+ if (IS_ERR(pxa_ohci->usb_host))
+ return dev_err_probe(&pdev->dev, PTR_ERR(pxa_ohci->usb_host),
+ "failed to get USB host GPIO\n");
for (i = 0; i < 3; ++i) {
char name[6];
@@ -512,6 +517,8 @@ static void ohci_hcd_pxa27x_remove(struct platform_device *pdev)
for (i = 0; i < 3; ++i)
pxa27x_ohci_set_vbus_power(pxa_ohci, i, false);
+ gpiod_put(pxa_ohci->usb_host);
+
usb_put_hcd(hcd);
}
--
2.42.0
^ permalink raw reply related
* [PATCH RFT v6 3/6] ARM: pxa: Convert Spitz CF power control to GPIO descriptors
From: Duje Mihanović @ 2023-10-06 13:44 UTC (permalink / raw)
To: Daniel Mack, Haojian Zhuang, Robert Jarzmik, Russell King,
Alan Stern, Greg Kroah-Hartman, Linus Walleij,
Bartosz Golaszewski, Andy Shevchenko, Dmitry Torokhov, Mark Brown
Cc: linux-arm-kernel, linux-kernel, linux-usb, linux-gpio,
linux-input, linux-spi, Duje Mihanović, Bartosz Golaszewski
In-Reply-To: <20231006-pxa-gpio-v6-0-981b4910d599@skole.hr>
Sharp's Spitz board still uses the legacy GPIO interface for controlling
the power supply to its CF and SD card slots.
Convert it to use the GPIO descriptor interface.
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Signed-off-by: Duje Mihanović <duje.mihanovic@skole.hr>
---
arch/arm/mach-pxa/spitz.c | 20 ++++++++++++++++++--
1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-pxa/spitz.c b/arch/arm/mach-pxa/spitz.c
index 1fb4102ea39e..8e9301cd0c93 100644
--- a/arch/arm/mach-pxa/spitz.c
+++ b/arch/arm/mach-pxa/spitz.c
@@ -133,6 +133,10 @@ static unsigned long spitz_pin_config[] __initdata = {
* Scoop GPIO expander
******************************************************************************/
#if defined(CONFIG_SHARP_SCOOP) || defined(CONFIG_SHARP_SCOOP_MODULE)
+GPIO_LOOKUP_SINGLE(spitz_card_pwr_ctrl_gpio_table, "pxa2xx-mci.0",
+ "sharp-scoop", SPITZ_GPIO_CF_POWER, "cf_power",
+ GPIO_ACTIVE_HIGH);
+
/* SCOOP Device #1 */
static struct resource spitz_scoop_1_resources[] = {
[0] = {
@@ -190,6 +194,7 @@ struct platform_device spitz_scoop_2_device = {
static void __init spitz_scoop_init(void)
{
platform_device_register(&spitz_scoop_1_device);
+ gpiod_add_lookup_table(&spitz_card_pwr_ctrl_gpio_table);
/* Akita doesn't have the second SCOOP chip */
if (!machine_is_akita())
@@ -201,9 +206,18 @@ static void __maybe_unused spitz_card_pwr_ctrl(uint8_t enable, uint8_t new_cpr)
{
unsigned short cpr;
unsigned long flags;
+ struct gpio_desc *cf_power;
+
+ cf_power = gpiod_get(&pxa_device_mci.dev, "cf_power", GPIOD_ASIS);
+ if (IS_ERR(cf_power)) {
+ dev_err(&pxa_device_mci.dev,
+ "failed to get power control GPIO with %ld\n",
+ PTR_ERR(cf_power));
+ return;
+ }
if (new_cpr & 0x7) {
- gpio_set_value(SPITZ_GPIO_CF_POWER, 1);
+ gpiod_direction_output(cf_power, 1);
mdelay(5);
}
@@ -222,8 +236,10 @@ static void __maybe_unused spitz_card_pwr_ctrl(uint8_t enable, uint8_t new_cpr)
if (!(cpr & 0x7)) {
mdelay(1);
- gpio_set_value(SPITZ_GPIO_CF_POWER, 0);
+ gpiod_direction_output(cf_power, 0);
}
+
+ gpiod_put(cf_power);
}
#else
--
2.42.0
^ permalink raw reply related
* [PATCH RFT v6 4/6] ARM: pxa: Convert reset driver to GPIO descriptors
From: Duje Mihanović @ 2023-10-06 13:44 UTC (permalink / raw)
To: Daniel Mack, Haojian Zhuang, Robert Jarzmik, Russell King,
Alan Stern, Greg Kroah-Hartman, Linus Walleij,
Bartosz Golaszewski, Andy Shevchenko, Dmitry Torokhov, Mark Brown
Cc: linux-arm-kernel, linux-kernel, linux-usb, linux-gpio,
linux-input, linux-spi, Duje Mihanović, Bartosz Golaszewski
In-Reply-To: <20231006-pxa-gpio-v6-0-981b4910d599@skole.hr>
The PXA reset driver still uses the legacy GPIO interface for
configuring and asserting the reset pin.
Convert it to use the GPIO descriptor interface.
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Signed-off-by: Duje Mihanović <duje.mihanovic@skole.hr>
---
arch/arm/mach-pxa/reset.c | 39 +++++++++++++--------------------------
arch/arm/mach-pxa/reset.h | 3 +--
arch/arm/mach-pxa/spitz.c | 6 +++++-
3 files changed, 19 insertions(+), 29 deletions(-)
diff --git a/arch/arm/mach-pxa/reset.c b/arch/arm/mach-pxa/reset.c
index 27293549f8ad..08bc104b9411 100644
--- a/arch/arm/mach-pxa/reset.c
+++ b/arch/arm/mach-pxa/reset.c
@@ -2,7 +2,7 @@
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/delay.h>
-#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
#include <linux/io.h>
#include <asm/proc-fns.h>
#include <asm/system_misc.h>
@@ -14,33 +14,20 @@
static void do_hw_reset(void);
-static int reset_gpio = -1;
+static struct gpio_desc *reset_gpio;
-int init_gpio_reset(int gpio, int output, int level)
+int init_gpio_reset(int output, int level)
{
- int rc;
-
- rc = gpio_request(gpio, "reset generator");
- if (rc) {
- printk(KERN_ERR "Can't request reset_gpio\n");
- goto out;
+ reset_gpio = gpiod_get(NULL, "reset", GPIOD_ASIS);
+ if (IS_ERR(reset_gpio)) {
+ pr_err("Can't request reset_gpio: %pe\n", reset_gpio);
+ return PTR_ERR(reset_gpio);
}
if (output)
- rc = gpio_direction_output(gpio, level);
+ return gpiod_direction_output(reset_gpio, level);
else
- rc = gpio_direction_input(gpio);
- if (rc) {
- printk(KERN_ERR "Can't configure reset_gpio\n");
- gpio_free(gpio);
- goto out;
- }
-
-out:
- if (!rc)
- reset_gpio = gpio;
-
- return rc;
+ return gpiod_direction_input(reset_gpio);
}
/*
@@ -50,16 +37,16 @@ int init_gpio_reset(int gpio, int output, int level)
*/
static void do_gpio_reset(void)
{
- BUG_ON(reset_gpio == -1);
+ BUG_ON(IS_ERR(reset_gpio));
/* drive it low */
- gpio_direction_output(reset_gpio, 0);
+ gpiod_direction_output(reset_gpio, 0);
mdelay(2);
/* rising edge or drive high */
- gpio_set_value(reset_gpio, 1);
+ gpiod_set_value(reset_gpio, 1);
mdelay(2);
/* falling edge */
- gpio_set_value(reset_gpio, 0);
+ gpiod_set_value(reset_gpio, 0);
/* give it some time */
mdelay(10);
diff --git a/arch/arm/mach-pxa/reset.h b/arch/arm/mach-pxa/reset.h
index 963dd190bc13..5864f61a0e94 100644
--- a/arch/arm/mach-pxa/reset.h
+++ b/arch/arm/mach-pxa/reset.h
@@ -13,10 +13,9 @@ extern void pxa_register_wdt(unsigned int reset_status);
/**
* init_gpio_reset() - register GPIO as reset generator
- * @gpio: gpio nr
* @output: set gpio as output instead of input during normal work
* @level: output level
*/
-extern int init_gpio_reset(int gpio, int output, int level);
+extern int init_gpio_reset(int output, int level);
#endif /* __ASM_ARCH_RESET_H */
diff --git a/arch/arm/mach-pxa/spitz.c b/arch/arm/mach-pxa/spitz.c
index 8e9301cd0c93..554a4b9782c5 100644
--- a/arch/arm/mach-pxa/spitz.c
+++ b/arch/arm/mach-pxa/spitz.c
@@ -1026,9 +1026,13 @@ static void spitz_restart(enum reboot_mode mode, const char *cmd)
spitz_poweroff();
}
+GPIO_LOOKUP_SINGLE(spitz_reset_gpio_table, NULL, "pxa-gpio",
+ SPITZ_GPIO_ON_RESET, "reset", GPIO_ACTIVE_HIGH);
+
static void __init spitz_init(void)
{
- init_gpio_reset(SPITZ_GPIO_ON_RESET, 1, 0);
+ gpiod_add_lookup_table(&spitz_reset_gpio_table);
+ init_gpio_reset(1, 0);
pm_power_off = spitz_poweroff;
PMCR = 0x00;
--
2.42.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox