[RFC,v2,2/2] samples: rust: Add USB sample bindings

Message ID 20231027003504.146703-3-yakoyoku@gmail.com
State New
Headers
Series rust: crates in other kernel directories |

Commit Message

Martin Rodriguez Reboredo Oct. 27, 2023, 12:34 a.m. UTC
  This is a demonstration of the capabilities of doing bindings with
subsystems that may or may not be statically linked.

Signed-off-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com>
---
 drivers/usb/core/Kconfig        |  7 +++++++
 drivers/usb/core/Makefile       |  3 +++
 drivers/usb/core/usb.rs         | 13 +++++++++++++
 samples/rust/Kconfig            | 10 ++++++++++
 samples/rust/Makefile           |  3 +++
 samples/rust/rust_usb_simple.rs | 22 ++++++++++++++++++++++
 6 files changed, 58 insertions(+)
 create mode 100644 drivers/usb/core/usb.rs
 create mode 100644 samples/rust/rust_usb_simple.rs
  

Comments

Martin Rodriguez Reboredo Oct. 27, 2023, 3:21 a.m. UTC | #1
This was missing in the patch, but you get the point...

diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index c41eaab4ddb2..845cdd856981 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -10,6 +10,7 @@
 #include <linux/errname.h>
 #include <linux/slab.h>
 #include <linux/refcount.h>
+#include <linux/usb.h>
 #include <linux/wait.h>
 #include <linux/sched.h>
 #include <linux/workqueue.h>
  
Greg KH Oct. 27, 2023, 7:15 a.m. UTC | #2
On Thu, Oct 26, 2023 at 09:34:51PM -0300, Martin Rodriguez Reboredo wrote:
> This is a demonstration of the capabilities of doing bindings with
> subsystems that may or may not be statically linked.
> 
> Signed-off-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com>
> ---
>  drivers/usb/core/Kconfig        |  7 +++++++
>  drivers/usb/core/Makefile       |  3 +++
>  drivers/usb/core/usb.rs         | 13 +++++++++++++
>  samples/rust/Kconfig            | 10 ++++++++++
>  samples/rust/Makefile           |  3 +++
>  samples/rust/rust_usb_simple.rs | 22 ++++++++++++++++++++++
>  6 files changed, 58 insertions(+)
>  create mode 100644 drivers/usb/core/usb.rs
>  create mode 100644 samples/rust/rust_usb_simple.rs
> 
> diff --git a/drivers/usb/core/Kconfig b/drivers/usb/core/Kconfig
> index 351ede4b5de2..4b5604282129 100644
> --- a/drivers/usb/core/Kconfig
> +++ b/drivers/usb/core/Kconfig
> @@ -116,3 +116,10 @@ config USB_AUTOSUSPEND_DELAY
>  	  The default value Linux has always had is 2 seconds.  Change
>  	  this value if you want a different delay and cannot modify
>  	  the command line or module parameter.
> +
> +config USB_RUST
> +	bool "Rust USB bindings"
> +	depends on USB && RUST
> +	default n

Nit, "n" is the default, this line is not needed.

Also, if you want to get really picky, _which_ USB is this for, the
"host" apis (you plug a USB device into a Linux maching), or the
"gadget" apis (i.e. Linux is running in the device that you plug into a
USB host)?  Linux supports both :)

> +	help
> +	  Enables Rust bindings for USB.
> diff --git a/drivers/usb/core/Makefile b/drivers/usb/core/Makefile
> index 7d338e9c0657..00e116913591 100644
> --- a/drivers/usb/core/Makefile
> +++ b/drivers/usb/core/Makefile
> @@ -11,6 +11,7 @@ usbcore-y += phy.o port.o
>  usbcore-$(CONFIG_OF)		+= of.o
>  usbcore-$(CONFIG_USB_PCI)		+= hcd-pci.o
>  usbcore-$(CONFIG_ACPI)		+= usb-acpi.o
> +usbcore-$(CONFIG_USB_RUST)		+= libusb.rlib
>  
>  ifdef CONFIG_USB_ONBOARD_HUB
>  usbcore-y			+= ../misc/onboard_usb_hub_pdevs.o
> @@ -18,4 +19,6 @@ endif
>  
>  obj-$(CONFIG_USB)		+= usbcore.o
>  
> +rust-libs			:= ./usb
> +
>  obj-$(CONFIG_USB_LEDS_TRIGGER_USBPORT)	+= ledtrig-usbport.o
> diff --git a/drivers/usb/core/usb.rs b/drivers/usb/core/usb.rs
> new file mode 100644
> index 000000000000..3f7ad02153f5
> --- /dev/null
> +++ b/drivers/usb/core/usb.rs
> @@ -0,0 +1,13 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! USB devices and drivers.
> +//!
> +//! C header: [`include/linux/usb.h`](../../../../include/linux/usb.h)
> +
> +use kernel::bindings;
> +
> +/// Check if USB is disabled.
> +pub fn disabled() -> bool {
> +    // SAFETY: FFI call.
> +    unsafe { bindings::usb_disabled() != 0 }
> +}
> diff --git a/samples/rust/Kconfig b/samples/rust/Kconfig
> index b0f74a81c8f9..12116f6fb526 100644
> --- a/samples/rust/Kconfig
> +++ b/samples/rust/Kconfig
> @@ -30,6 +30,16 @@ config SAMPLE_RUST_PRINT
>  
>  	  If unsure, say N.
>  
> +config SAMPLE_RUST_USB_SIMPLE
> +	tristate "USB simple device driver"
> +	help
> +	  This option builds the Rust USB simple driver sample.
> +
> +	  To compile this as a module, choose M here:
> +	  the module will be called rust_usb_simple.
> +
> +	  If unsure, say N.
> +
>  config SAMPLE_RUST_HOSTPROGS
>  	bool "Host programs"
>  	help
> diff --git a/samples/rust/Makefile b/samples/rust/Makefile
> index 03086dabbea4..f1ab58a9ecdd 100644
> --- a/samples/rust/Makefile
> +++ b/samples/rust/Makefile
> @@ -2,5 +2,8 @@
>  
>  obj-$(CONFIG_SAMPLE_RUST_MINIMAL)		+= rust_minimal.o
>  obj-$(CONFIG_SAMPLE_RUST_PRINT)			+= rust_print.o
> +obj-$(CONFIG_SAMPLE_RUST_USB_SIMPLE)		+= rust_usb_simple.o
> +
> +rust-libs					:= ../../drivers/usb/core/usb
>  
>  subdir-$(CONFIG_SAMPLE_RUST_HOSTPROGS)		+= hostprogs
> diff --git a/samples/rust/rust_usb_simple.rs b/samples/rust/rust_usb_simple.rs
> new file mode 100644
> index 000000000000..3523f81d5eb8
> --- /dev/null
> +++ b/samples/rust/rust_usb_simple.rs
> @@ -0,0 +1,22 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Rust USB sample.
> +
> +use kernel::prelude::*;
> +
> +module! {
> +    type: UsbSimple,
> +    name: "rust_usb_simple",
> +    author: "Martin Rodriguez Reboredo",
> +    description: "Rust USB sample",
> +    license: "GPL v2",
> +}
> +
> +struct UsbSimple;

"USBSimple" please.

> +
> +impl kernel::Module for UsbSimple {
> +    fn init(_module: &'static ThisModule) -> Result<Self> {
> +        pr_info!("usb enabled: {}", !usb::disabled());
> +        Ok(UsbSimple)
> +    }
> +}

I know this is just a fake patch to test the bindings logic, so sorry
for the noise, just wanted to get terminology right :)

thanks,

greg k-h
  
Martin Rodriguez Reboredo Oct. 27, 2023, 1:07 p.m. UTC | #3
On 10/27/23 04:15, Greg Kroah-Hartman wrote:
> On Thu, Oct 26, 2023 at 09:34:51PM -0300, Martin Rodriguez Reboredo wrote:
>> [...]
>> +
>> +config USB_RUST
>> +	bool "Rust USB bindings"
>> +	depends on USB && RUST
>> +	default n
> 
> Nit, "n" is the default, this line is not needed.
> 
> Also, if you want to get really picky, _which_ USB is this for, the
> "host" apis (you plug a USB device into a Linux maching), or the
> "gadget" apis (i.e. Linux is running in the device that you plug into a
> USB host)?  Linux supports both :)

Nice one! I'll probably target host APIs as of now due to what I have in
my hands.

>> +	help
>> +	  Enables Rust bindings for USB.
>> [...]
>> +//! Rust USB sample.
>> +
>> +use kernel::prelude::*;
>> +
>> +module! {
>> +    type: UsbSimple,
>> +    name: "rust_usb_simple",
>> +    author: "Martin Rodriguez Reboredo",
>> +    description: "Rust USB sample",
>> +    license: "GPL v2",
>> +}
>> +
>> +struct UsbSimple;
> 
> "USBSimple" please.
> 
>> +
>> +impl kernel::Module for UsbSimple {
>> +    fn init(_module: &'static ThisModule) -> Result<Self> {
>> +        pr_info!("usb enabled: {}", !usb::disabled());
>> +        Ok(UsbSimple)
>> +    }
>> +}
> 
> I know this is just a fake patch to test the bindings logic, so sorry
> for the noise, just wanted to get terminology right :)
> 
> thanks,
> 
> greg k-h

No, you are right. Because Rust uses upper camel case for struct names
there's the tendency of lowering initialisms, so you can either leave it
as `USBSimple` or as `UsbSimple`. When I publish my USB host bindings
I'll take your comment into account for sure.
  
Greg KH Oct. 27, 2023, 1:08 p.m. UTC | #4
On Thu, Oct 26, 2023 at 09:34:51PM -0300, Martin Rodriguez Reboredo wrote:
> This is a demonstration of the capabilities of doing bindings with
> subsystems that may or may not be statically linked.
> 
> Signed-off-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com>
> ---
>  drivers/usb/core/Kconfig        |  7 +++++++
>  drivers/usb/core/Makefile       |  3 +++
>  drivers/usb/core/usb.rs         | 13 +++++++++++++
>  samples/rust/Kconfig            | 10 ++++++++++
>  samples/rust/Makefile           |  3 +++
>  samples/rust/rust_usb_simple.rs | 22 ++++++++++++++++++++++
>  6 files changed, 58 insertions(+)
>  create mode 100644 drivers/usb/core/usb.rs
>  create mode 100644 samples/rust/rust_usb_simple.rs
> 

Hi,

This is the friendly patch-bot of Greg Kroah-Hartman.  You have sent him
a patch that has triggered this response.  He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created.  Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.

You are receiving this message because of the following common error(s)
as indicated below:

- This looks like a new version of a previously submitted patch, but you
  did not list below the --- line any changes from the previous version.
  Please read the section entitled "The canonical patch format" in the
  kernel file, Documentation/process/submitting-patches.rst for what
  needs to be done here to properly describe this.

If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.

thanks,

greg k-h's patch email bot
  

Patch

diff --git a/drivers/usb/core/Kconfig b/drivers/usb/core/Kconfig
index 351ede4b5de2..4b5604282129 100644
--- a/drivers/usb/core/Kconfig
+++ b/drivers/usb/core/Kconfig
@@ -116,3 +116,10 @@  config USB_AUTOSUSPEND_DELAY
 	  The default value Linux has always had is 2 seconds.  Change
 	  this value if you want a different delay and cannot modify
 	  the command line or module parameter.
+
+config USB_RUST
+	bool "Rust USB bindings"
+	depends on USB && RUST
+	default n
+	help
+	  Enables Rust bindings for USB.
diff --git a/drivers/usb/core/Makefile b/drivers/usb/core/Makefile
index 7d338e9c0657..00e116913591 100644
--- a/drivers/usb/core/Makefile
+++ b/drivers/usb/core/Makefile
@@ -11,6 +11,7 @@  usbcore-y += phy.o port.o
 usbcore-$(CONFIG_OF)		+= of.o
 usbcore-$(CONFIG_USB_PCI)		+= hcd-pci.o
 usbcore-$(CONFIG_ACPI)		+= usb-acpi.o
+usbcore-$(CONFIG_USB_RUST)		+= libusb.rlib
 
 ifdef CONFIG_USB_ONBOARD_HUB
 usbcore-y			+= ../misc/onboard_usb_hub_pdevs.o
@@ -18,4 +19,6 @@  endif
 
 obj-$(CONFIG_USB)		+= usbcore.o
 
+rust-libs			:= ./usb
+
 obj-$(CONFIG_USB_LEDS_TRIGGER_USBPORT)	+= ledtrig-usbport.o
diff --git a/drivers/usb/core/usb.rs b/drivers/usb/core/usb.rs
new file mode 100644
index 000000000000..3f7ad02153f5
--- /dev/null
+++ b/drivers/usb/core/usb.rs
@@ -0,0 +1,13 @@ 
+// SPDX-License-Identifier: GPL-2.0
+
+//! USB devices and drivers.
+//!
+//! C header: [`include/linux/usb.h`](../../../../include/linux/usb.h)
+
+use kernel::bindings;
+
+/// Check if USB is disabled.
+pub fn disabled() -> bool {
+    // SAFETY: FFI call.
+    unsafe { bindings::usb_disabled() != 0 }
+}
diff --git a/samples/rust/Kconfig b/samples/rust/Kconfig
index b0f74a81c8f9..12116f6fb526 100644
--- a/samples/rust/Kconfig
+++ b/samples/rust/Kconfig
@@ -30,6 +30,16 @@  config SAMPLE_RUST_PRINT
 
 	  If unsure, say N.
 
+config SAMPLE_RUST_USB_SIMPLE
+	tristate "USB simple device driver"
+	help
+	  This option builds the Rust USB simple driver sample.
+
+	  To compile this as a module, choose M here:
+	  the module will be called rust_usb_simple.
+
+	  If unsure, say N.
+
 config SAMPLE_RUST_HOSTPROGS
 	bool "Host programs"
 	help
diff --git a/samples/rust/Makefile b/samples/rust/Makefile
index 03086dabbea4..f1ab58a9ecdd 100644
--- a/samples/rust/Makefile
+++ b/samples/rust/Makefile
@@ -2,5 +2,8 @@ 
 
 obj-$(CONFIG_SAMPLE_RUST_MINIMAL)		+= rust_minimal.o
 obj-$(CONFIG_SAMPLE_RUST_PRINT)			+= rust_print.o
+obj-$(CONFIG_SAMPLE_RUST_USB_SIMPLE)		+= rust_usb_simple.o
+
+rust-libs					:= ../../drivers/usb/core/usb
 
 subdir-$(CONFIG_SAMPLE_RUST_HOSTPROGS)		+= hostprogs
diff --git a/samples/rust/rust_usb_simple.rs b/samples/rust/rust_usb_simple.rs
new file mode 100644
index 000000000000..3523f81d5eb8
--- /dev/null
+++ b/samples/rust/rust_usb_simple.rs
@@ -0,0 +1,22 @@ 
+// SPDX-License-Identifier: GPL-2.0
+
+//! Rust USB sample.
+
+use kernel::prelude::*;
+
+module! {
+    type: UsbSimple,
+    name: "rust_usb_simple",
+    author: "Martin Rodriguez Reboredo",
+    description: "Rust USB sample",
+    license: "GPL v2",
+}
+
+struct UsbSimple;
+
+impl kernel::Module for UsbSimple {
+    fn init(_module: &'static ThisModule) -> Result<Self> {
+        pr_info!("usb enabled: {}", !usb::disabled());
+        Ok(UsbSimple)
+    }
+}