Updates for turris-mox-rwtm driver
Marek Behún
maintainercover Updates for turris-mox-rwtm driver
Hi Gregory, Arnd, Andy et al.
this is a series of fixes, refactors and cleanups for the turris-mox-rwtm driver.
Marek
Marek Behún (19):
firmware: turris-mox-rwtm: Do not complete if there are no waiters
firmware: turris-mox-rwtm: Fix checking return value of
wait_for_completion_timeout()
firmware: turris-mox-rwtm: Use PAGE_SIZE instead of hardcoded 4096
firmware: turris-mox-rwtm: Use ETH_ALEN instead of hardcoded 6
firmware: turris-mox-rwtm: Use the boolean type where appropriate
firmware: turris-mox-rwtm: Hide signature related constants behind
macros
firmware: turris-mox-rwtm: Fix driver includes
firmware: turris-mox-rwtm: Use kobj_type's default_groups member for
automatic sysfs files creation
firmware: turris-mox-rwtm: Simplify debugfs code
firmware: turris-mox-rwtm: Simplify driver kobject code
firmware: turris-mox-rwtm: Return true error code if kobject_add()
fails
firmware: turris-mox-rwtm: Convert rest to devm_* and get rid of
driver .remove()
firmware: turris-mox-rwtm: Use dev_err_probe() where possible
firmware: turris-mox-rwtm: Rearrange probe calls
firmware: turris-mox-rwtm: Drop redundant device pointer
firmware: turris-mox-rwtm: Use devm_mutex_init() instead of
mutex_init()
firmware: turris-mox-rwtm: Use container_of() instead of hwrng .priv
member
firmware: turris-mox-rwtm: Deduplicate command execution code
firmware: turris-mox-rwtm: Use EOPNOTSUPP instead of ENOSYS
drivers/firmware/turris-mox-rwtm.c | 390 +++++++++++++---------------- 1 file changed, 172 insertions(+), 218 deletions(-)
Signature · 1 line
2.44.2
Marek Behún
maintainerPATCH 1/19 firmware: turris-mox-rwtm: Do not complete if there are no waiters
Do not complete the command done completion if there are no waiters. This can happen if a wait_for_completion timed out or was interrupted.
--- drivers/firmware/turris-mox-rwtm.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
drivers/firmware/turris-mox-rwtm.c +4 -1
@@ -2,7 +2,7 @@/* * Turris Mox rWTM firmware driver * -* Copyright (C) 2019 Marek Behún <[email protected]> +* Copyright (C) 2019, 2024 Marek Behún <[email protected]> */ #include <linux/armada-37xx-rwtm-mailbox.h>
@@ -174,6 +174,9 @@ static void mox_rwtm_rx_callback(struct mbox_client *cl, void *data)struct mox_rwtm *rwtm =dev_get_drvdata (cl->dev);struct armada_37xx_rwtm_rx_msg *msg = data; +if (completion_done (&rwtm->cmd_done)) +return ; + rwtm->reply = *msg;complete (&rwtm->cmd_done); }
Signature · 1 line
2.44.2
Marek Behún
maintainerPATCH 2/19 firmware: turris-mox-rwtm: Fix checking return value of wait_for_completion_timeout()
In reply to Marek BehúnThe wait_for_completion_timeout() function returns 0 if timed out, and a positive value if completed. Fix the usage of this function.
--- drivers/firmware/turris-mox-rwtm.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-)
drivers/firmware/turris-mox-rwtm.c +6 -9
@@ -202,9 +202,8 @@ static int mox_get_board_info(struct mox_rwtm *rwtm)if (ret <0 )return ret; - ret =wait_for_completion_timeout (&rwtm->cmd_done,HZ /2 ); -if (ret <0 ) -return ret; +if (!wait_for_completion_timeout (&rwtm->cmd_done,HZ /2 )) +return -ETIMEDOUT ; ret =mox_get_status (MBOX_CMD_BOARD_INFO , reply->retval);if (ret == -ENODATA ) {
@@ -238,9 +237,8 @@ static int mox_get_board_info(struct mox_rwtm *rwtm)if (ret <0 )return ret; - ret =wait_for_completion_timeout (&rwtm->cmd_done,HZ /2 ); -if (ret <0 ) -return ret; +if (!wait_for_completion_timeout (&rwtm->cmd_done,HZ /2 )) +return -ETIMEDOUT ; ret =mox_get_status (MBOX_CMD_ECDSA_PUB_KEY , reply->retval);if (ret == -ENODATA ) {
@@ -277,9 +275,8 @@ static int check_get_random_support(struct mox_rwtm *rwtm)if (ret <0 )return ret; - ret =wait_for_completion_timeout (&rwtm->cmd_done,HZ /2 ); -if (ret <0 ) -return ret; +if (!wait_for_completion_timeout (&rwtm->cmd_done,HZ /2 )) +return -ETIMEDOUT ;return mox_get_status (MBOX_CMD_GET_RANDOM , rwtm->reply.retval); }
Signature · 1 line
2.44.2
Marek Behún
maintainerPATCH 3/19 firmware: turris-mox-rwtm: Use PAGE_SIZE instead of hardcoded 4096
In reply to Marek BehúnThe 4096 bytes limit in mox_hwrng_read() is due to the DMA buffer being allocated to one PAGE_SIZE bytes. The PAGE_SIZE macro is used when allocating the buffer, use it in mox_hwrng_read() as well.
--- drivers/firmware/turris-mox-rwtm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/firmware/turris-mox-rwtm.c +2 -2
@@ -287,8 +287,8 @@ static int mox_hwrng_read(struct hwrng *rng, void *data, size_t max, bool wait)struct armada_37xx_rwtm_tx_msg msg;int ret; -if (max >4096 ) - max =4096 ; +if (max >PAGE_SIZE ) + max =PAGE_SIZE ; msg.command =MBOX_CMD_GET_RANDOM ; msg.args[0 ] =1 ;
Signature · 1 line
2.44.2
Marek Behún
maintainerPATCH 4/19 firmware: turris-mox-rwtm: Use ETH_ALEN instead of hardcoded 6
In reply to Marek BehúnUse the ETH_ALEN macro instead of hardcoded 6 for MAC address length.
--- drivers/firmware/turris-mox-rwtm.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/firmware/turris-mox-rwtm.c +2 -1
@@ -10,6 +10,7 @@#include <linux/debugfs.h> #include <linux/dma-mapping.h> #include <linux/hw_random.h> +#include <linux/if_ether.h> #include <linux/mailbox_client.h> #include <linux/module.h> #include <linux/mutex.h>
@@ -65,7 +66,7 @@ struct mox_rwtm {int has_board_info;u64 serial_number;int board_version, ram_size; -u8 mac_address1[6 ], mac_address2[6 ]; +u8 mac_address1[ETH_ALEN ], mac_address2[ETH_ALEN ];/* public key burned in eFuse */ int has_pubkey;
Signature · 1 line
2.44.2
Marek Behún
maintainerPATCH 5/19 firmware: turris-mox-rwtm: Use the boolean type where appropriate
In reply to Marek BehúnUse the boolean type for has_board_info, has_pubkey and last_sig_done members of the driver's private structure.
--- drivers/firmware/turris-mox-rwtm.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-)
drivers/firmware/turris-mox-rwtm.c +8 -7
@@ -17,6 +17,7 @@#include <linux/of.h> #include <linux/platform_device.h> #include <linux/slab.h> +#include <linux/types.h> #define DRIVER_NAME "turris-mox-rwtm"
@@ -63,13 +64,13 @@ struct mox_rwtm {struct completion cmd_done;/* board information */ -int has_board_info; +bool has_board_info;u64 serial_number;int board_version, ram_size;u8 mac_address1[ETH_ALEN ], mac_address2[ETH_ALEN ];/* public key burned in eFuse */ -int has_pubkey; +bool has_pubkey;u8 pubkey[135 ];#ifdef CONFIG_DEBUG_FS
@@ -81,7 +82,7 @@ struct mox_rwtm {*/ struct dentry *debugfs_root;u32 last_sig[34 ]; -int last_sig_done; +bool last_sig_done;#endif };
@@ -225,7 +226,7 @@ static int mox_get_board_info(struct mox_rwtm *rwtm) reply->status[5]);reply_to_mac_addr (rwtm->mac_address2, reply->status[6 ], reply->status[7 ]); - rwtm->has_board_info =1 ; + rwtm->has_board_info = true;pr_info ("Turris Mox serial number %016llX\n" , rwtm->serial_number);
@@ -252,7 +253,7 @@ static int mox_get_board_info(struct mox_rwtm *rwtm) }else {u32 *s = reply->status; - rwtm->has_pubkey =1 ; + rwtm->has_pubkey = true; sprintf(rwtm->pubkey, "%06x%08x%08x%08x%08x%08x%08x%08x%08x%08x%08x%08x%08x%08x%08x%08x%08x", ret, s[0], s[1], s[2], s[3], s[4], s[5], s[6], s[7],
@@ -349,7 +350,7 @@ static ssize_t do_sign_read(struct file *file, char __user *buf, size_t len,/* 2 arrays of 17 32-bit words are 136 bytes */ ret =simple_read_from_buffer (buf, len, ppos, rwtm->last_sig,136 ); - rwtm->last_sig_done =0 ; + rwtm->last_sig_done = false;return ret; }
@@ -415,7 +416,7 @@ static ssize_t do_sign_write(struct file *file, const char __user *buf,*/ memcpy (rwtm->last_sig, rwtm->buf +68 ,136 );cpu_to_be32_array (rwtm->last_sig, rwtm->last_sig,34 ); - rwtm->last_sig_done =1 ; + rwtm->last_sig_done = true;mutex_unlock (&rwtm->busy);return len;
Signature · 1 line
2.44.2
Marek Behún
maintainerPATCH 6/19 firmware: turris-mox-rwtm: Hide signature related constants behind macros
In reply to Marek BehúnHide signature generation related constants behind macros instead of hardcoding the values.
Use SHA512_DIGEST_SIZE from crypto/sha2.h instead of hardcoded 64 as the message size.
--- drivers/firmware/turris-mox-rwtm.c | 34 +++++++++++++++++++----------- 1 file changed, 22 insertions(+), 12 deletions(-)
drivers/firmware/turris-mox-rwtm.c +22 -12
@@ -5,6 +5,7 @@* Copyright (C) 2019, 2024 Marek Behún <[email protected]> */ +#include <crypto/sha2.h> #include <linux/armada-37xx-rwtm-mailbox.h> #include <linux/completion.h> #include <linux/debugfs.h>
@@ -27,6 +28,12 @@* https://gitlab.labs.nic.cz/turris/mox-boot-builder/tree/master/wtmi. */ +#define MOX_ECC_NUMBER_WORDS 17 +#define MOX_ECC_NUMBER_LEN (MOX_ECC_NUMBER_WORDS * sizeof(u32)) + +#define MOX_ECC_SIGNATURE_WORDS (2 * MOX_ECC_NUMBER_WORDS) +#define MOX_ECC_SIGNATURE_LEN (MOX_ECC_NUMBER_WORDS * sizeof(u32)) +#define MBOX_STS_SUCCESS (0 << 30)#define MBOX_STS_FAIL (1 << 30)#define MBOX_STS_BADCMD (2 << 30)
@@ -81,7 +88,7 @@ struct mox_rwtm {* from userspace. */ struct dentry *debugfs_root; -u32 last_sig[34 ]; +u32 last_sig[MOX_ECC_SIGNATURE_WORDS ];bool last_sig_done;#endif };
@@ -342,14 +349,15 @@ static ssize_t do_sign_read(struct file *file, char __user *buf, size_t len,if (*ppos !=0 )return 0 ; -if (len <136 ) +if (len <MOX_ECC_SIGNATURE_LEN )return -EINVAL ;if (!rwtm->last_sig_done)return -ENODATA ;/* 2 arrays of 17 32-bit words are 136 bytes */ - ret =simple_read_from_buffer (buf, len, ppos, rwtm->last_sig,136 ); + ret =simple_read_from_buffer (buf, len, ppos, rwtm->last_sig, +MOX_ECC_SIGNATURE_LEN ); rwtm->last_sig_done = false;return ret;
@@ -364,8 +372,7 @@ static ssize_t do_sign_write(struct file *file, const char __user *buf,loff_t dummy =0 ;ssize_t ret; -/* the input is a SHA-512 hash, so exactly 64 bytes have to be read */ -if (len !=64 ) +if (len !=SHA512_DIGEST_SIZE )return -EINVAL ;/* if last result is not zero user has not read that information yet */
@@ -386,17 +393,18 @@ static ssize_t do_sign_write(struct file *file, const char __user *buf,* 3. Address of the buffer where ECDSA signature value S shall be * stored by the rWTM firmware. */ -memset (rwtm->buf,0 ,4 ); - ret =simple_write_to_buffer (rwtm->buf +4 ,64 , &dummy, buf, len); +memset (rwtm->buf,0 ,sizeof (u32)); + ret =simple_write_to_buffer (rwtm->buf +sizeof (u32), +SHA512_DIGEST_SIZE , &dummy, buf, len);if (ret <0 ) goto unlock_mutex; -be32_to_cpu_array (rwtm->buf, rwtm->buf,17 ); +be32_to_cpu_array (rwtm->buf, rwtm->buf,MOX_ECC_NUMBER_WORDS ); msg.command =MBOX_CMD_SIGN ; msg.args[0 ] =1 ; msg.args[1 ] = rwtm->buf_phys; - msg.args[2 ] = rwtm->buf_phys +68 ; - msg.args[3 ] = rwtm->buf_phys +2 *68 ; + msg.args[2 ] = rwtm->buf_phys +MOX_ECC_NUMBER_LEN ; + msg.args[3 ] = rwtm->buf_phys +2 *MOX_ECC_NUMBER_LEN ; ret =mbox_send_message (rwtm->mbox, &msg);if (ret <0 ) goto unlock_mutex;
@@ -414,8 +422,10 @@ static ssize_t do_sign_write(struct file *file, const char __user *buf,* computed by the rWTM firmware and convert their words from * LE to BE. */ -memcpy (rwtm->last_sig, rwtm->buf +68 ,136 ); -cpu_to_be32_array (rwtm->last_sig, rwtm->last_sig,34 ); +memcpy (rwtm->last_sig, rwtm->buf +MOX_ECC_NUMBER_LEN , +MOX_ECC_SIGNATURE_LEN ); +cpu_to_be32_array (rwtm->last_sig, rwtm->last_sig, +MOX_ECC_SIGNATURE_WORDS ); rwtm->last_sig_done = true;mutex_unlock (&rwtm->busy);
Signature · 1 line
2.44.2
Marek Behún
maintainerPATCH 7/19 firmware: turris-mox-rwtm: Fix driver includes
In reply to Marek BehúnDrop including of.h, include several other headers that are used but not included directly.
--- drivers/firmware/turris-mox-rwtm.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
drivers/firmware/turris-mox-rwtm.c +6 -1
@@ -8,16 +8,21 @@#include <crypto/sha2.h> #include <linux/armada-37xx-rwtm-mailbox.h> #include <linux/completion.h> +#include <linux/container_of.h> #include <linux/debugfs.h> +#include <linux/device.h> #include <linux/dma-mapping.h> +#include <linux/err.h> +#include <linux/fs.h> #include <linux/hw_random.h> #include <linux/if_ether.h> +#include <linux/kobject.h> #include <linux/mailbox_client.h> #include <linux/module.h> #include <linux/mutex.h> -#include <linux/of.h> #include <linux/platform_device.h> #include <linux/slab.h> +#include <linux/sysfs.h> #include <linux/types.h> #define DRIVER_NAME "turris-mox-rwtm"
Signature · 1 line
2.44.2
Marek Behún
maintainerPATCH 8/19 firmware: turris-mox-rwtm: Use kobj_type's default_groups member for automatic sysfs files creation
In reply to Marek BehúnPut the turris-mox-rwtm attribute group into the .default_groups member of the underlying kobject type and drop the manual sysfs_create_files() / sysfs_remove_files(). The kobject library will take care of this in it's internal code.
--- drivers/firmware/turris-mox-rwtm.c | 71 ++++++++++++++---------------- 1 file changed, 32 insertions(+), 39 deletions(-)
drivers/firmware/turris-mox-rwtm.c +32 -39
@@ -113,6 +113,36 @@ static inline struct mox_rwtm *to_rwtm(struct kobject *kobj) return container_of(kobj, struct mox_kobject, kobj)->rwtm; } +#define MOX_ATTR_RO (name, format, cat) \ +static ssize_t \ +name##_show(struct kobject *kobj, struct kobj_attribute *a, \ + char *buf) \ +{ \ + struct mox_rwtm *rwtm = to_rwtm(kobj); \ + if (!rwtm->has_##cat) \ + return -ENODATA; \ + return sprintf(buf, format, rwtm->name); \ +} \ +static struct kobj_attribute mox_attr_##name = __ATTR_RO(name) + +MOX_ATTR_RO (serial_number,"%016llX\n" , board_info); +MOX_ATTR_RO (board_version,"%i\n" , board_info); +MOX_ATTR_RO (ram_size,"%i\n" , board_info); +MOX_ATTR_RO (mac_address1,"%pM\n" , board_info); +MOX_ATTR_RO (mac_address2,"%pM\n" , board_info); +MOX_ATTR_RO (pubkey,"%s\n" , pubkey); + +static struct attribute *mox_rwtm_attrs[] = { + &mox_attr_serial_number.attr, + &mox_attr_board_version.attr, + &mox_attr_ram_size.attr, + &mox_attr_mac_address1.attr, + &mox_attr_mac_address2.attr, + &mox_attr_pubkey.attr, +NULL +}; +ATTRIBUTE_GROUPS (mox_rwtm); +static void mox_kobj_release (struct kobject *kobj) {kfree (to_rwtm (kobj)->kobj);
@@ -121,6 +151,7 @@ static void mox_kobj_release(struct kobject *kobj)static const struct kobj_type mox_kobj_ktype = { .release = mox_kobj_release, .sysfs_ops = &kobj_sysfs_ops, + .default_groups = mox_rwtm_groups, };static int mox_kobj_create (struct mox_rwtm *rwtm)
@@ -140,25 +171,6 @@ static int mox_kobj_create(struct mox_rwtm *rwtm)return 0 ; } -#define MOX_ATTR_RO (name, format, cat) \ -static ssize_t \ -name##_show(struct kobject *kobj, struct kobj_attribute *a, \ - char *buf) \ -{ \ - struct mox_rwtm *rwtm = to_rwtm(kobj); \ - if (!rwtm->has_##cat) \ - return -ENODATA; \ - return sprintf(buf, format, rwtm->name); \ -} \ -static struct kobj_attribute mox_attr_##name = __ATTR_RO(name) - -MOX_ATTR_RO (serial_number,"%016llX\n" , board_info); -MOX_ATTR_RO (board_version,"%i\n" , board_info); -MOX_ATTR_RO (ram_size,"%i\n" , board_info); -MOX_ATTR_RO (mac_address1,"%pM\n" , board_info); -MOX_ATTR_RO (mac_address2,"%pM\n" , board_info); -MOX_ATTR_RO (pubkey,"%s\n" , pubkey); -static int mox_get_status (enum mbox_cmd cmd,u32 retval) {if (MBOX_STS_CMD (retval) != cmd)
@@ -173,16 +185,6 @@ static int mox_get_status(enum mbox_cmd cmd, u32 retval)return MBOX_STS_VALUE (retval); } -static const struct attribute *mox_rwtm_attrs[] = { - &mox_attr_serial_number.attr, - &mox_attr_board_version.attr, - &mox_attr_ram_size.attr, - &mox_attr_mac_address1.attr, - &mox_attr_mac_address2.attr, - &mox_attr_pubkey.attr, -NULL -}; -static void mox_rwtm_rx_callback (struct mbox_client *cl,void *data) {struct mox_rwtm *rwtm =dev_get_drvdata (cl->dev);
@@ -507,12 +509,6 @@ static int turris_mox_rwtm_probe(struct platform_device *pdev)return ret; } - ret =sysfs_create_files (rwtm_to_kobj (rwtm), mox_rwtm_attrs); -if (ret <0 ) { -dev_err (dev,"Cannot create sysfs files!\n" ); - goto put_kobj; - } -platform_set_drvdata (pdev, rwtm);mutex_init (&rwtm->busy);
@@ -526,7 +522,7 @@ static int turris_mox_rwtm_probe(struct platform_device *pdev)if (ret != -EPROBE_DEFER )dev_err (dev,"Cannot request mailbox channel: %i\n" , ret); - goto remove_files; + goto put_kobj; }init_completion (&rwtm->cmd_done);
@@ -564,8 +560,6 @@ static int turris_mox_rwtm_probe(struct platform_device *pdev) free_channel:mbox_free_channel (rwtm->mbox); -remove_files: -sysfs_remove_files (rwtm_to_kobj (rwtm), mox_rwtm_attrs); put_kobj:kobject_put (rwtm_to_kobj (rwtm));return ret;
@@ -576,7 +570,6 @@ static void turris_mox_rwtm_remove(struct platform_device *pdev)struct mox_rwtm *rwtm =platform_get_drvdata (pdev);rwtm_unregister_debugfs (rwtm); -sysfs_remove_files (rwtm_to_kobj (rwtm), mox_rwtm_attrs);kobject_put (rwtm_to_kobj (rwtm));mbox_free_channel (rwtm->mbox); }
Signature · 1 line
2.44.2
Marek Behún
maintainerPATCH 9/19 firmware: turris-mox-rwtm: Simplify debugfs code
In reply to Marek BehúnSimplify debugfs code: do not check for errors, as debugfs errors should be ignored, and use devm action for dropping the debugfs directory.
--- drivers/firmware/turris-mox-rwtm.c | 44 ++++++++---------------------- 1 file changed, 11 insertions(+), 33 deletions(-)
drivers/firmware/turris-mox-rwtm.c +11 -33
@@ -92,7 +92,6 @@ struct mox_rwtm {* It should be rewritten via crypto API once akcipher API is available * from userspace. */ -struct dentry *debugfs_root;u32 last_sig[MOX_ECC_SIGNATURE_WORDS ];bool last_sig_done;#endif
@@ -450,39 +449,23 @@ static const struct file_operations do_sign_fops = { .llseek = no_llseek, }; -static int rwtm_register_debugfs (struct mox_rwtm *rwtm) +static void rwtm_debugfs_release (void *root) { -struct dentry *root, *entry; - - root =debugfs_create_dir ("turris-mox-rwtm" ,NULL ); - -if (IS_ERR (root)) -return PTR_ERR (root); - - entry =debugfs_create_file_unsafe ("do_sign" ,0600 , root, rwtm, - &do_sign_fops); -if (IS_ERR (entry)) - goto err_remove; - - rwtm->debugfs_root = root; - -return 0 ; -err_remove:debugfs_remove_recursive (root); -return PTR_ERR (entry); } -static void rwtm_unregister_debugfs (struct mox_rwtm *rwtm) +static void rwtm_register_debugfs (struct mox_rwtm *rwtm) { -debugfs_remove_recursive (rwtm->debugfs_root); +struct dentry *root; + + root =debugfs_create_dir ("turris-mox-rwtm" ,NULL ); + +devm_add_action_or_reset (rwtm->dev, rwtm_debugfs_release, root); + +debugfs_create_file_unsafe ("do_sign" ,0600 , root, rwtm, &do_sign_fops); }#else -static inline int rwtm_register_debugfs (struct mox_rwtm *rwtm) -{ -return 0 ; -} - -static inline void rwtm_unregister_debugfs (struct mox_rwtm *rwtm) +static inline void rwtm_register_debugfs (struct mox_rwtm *rwtm) { }#endif
@@ -548,11 +531,7 @@ static int turris_mox_rwtm_probe(struct platform_device *pdev) goto free_channel; } - ret =rwtm_register_debugfs (rwtm); -if (ret <0 ) { -dev_err (dev,"Failed creating debugfs entries: %i\n" , ret); - goto free_channel; - } +rwtm_register_debugfs (rwtm);dev_info (dev,"HWRNG successfully registered\n" );
@@ -569,7 +548,6 @@ static void turris_mox_rwtm_remove(struct platform_device *pdev) {struct mox_rwtm *rwtm =platform_get_drvdata (pdev); -rwtm_unregister_debugfs (rwtm);kobject_put (rwtm_to_kobj (rwtm));mbox_free_channel (rwtm->mbox); }
Signature · 1 line
2.44.2
Marek Behún
maintainerPATCH 10/19 firmware: turris-mox-rwtm: Simplify driver kobject code
In reply to Marek BehúnDrop the mox_kobject wrapper that needs to be allocated, instead put the kobject directly into the driver private structure. This allows us to drop one kzalloc() call.
--- drivers/firmware/turris-mox-rwtm.c | 36 ++++++++---------------------- 1 file changed, 9 insertions(+), 27 deletions(-)
drivers/firmware/turris-mox-rwtm.c +9 -27
@@ -21,7 +21,6 @@#include <linux/module.h> #include <linux/mutex.h> #include <linux/platform_device.h> -#include <linux/slab.h> #include <linux/sysfs.h> #include <linux/types.h>
@@ -58,13 +57,11 @@ enum mbox_cmd { MBOX_CMD_OTP_WRITE = 8, }; -struct mox_kobject ; -struct mox_rwtm {struct device *dev;struct mbox_client mbox_client;struct mbox_chan *mbox; -struct mox_kobject *kobj; +struct kobject kobj;struct hwrng hwrng;struct armada_37xx_rwtm_rx_msg reply;
@@ -97,19 +94,9 @@ struct mox_rwtm {#endif }; -struct mox_kobject { -struct kobject kobj; -struct mox_rwtm *rwtm; -}; - -static inline struct kobject *rwtm_to_kobj (struct mox_rwtm *rwtm) -{ -return &rwtm->kobj->kobj; -} -static inline struct mox_rwtm *to_rwtm (struct kobject *kobj) { - return container_of(kobj, struct mox_kobject, kobj)->rwtm; + return container_of(kobj, struct mox_rwtm, kobj); }#define MOX_ATTR_RO (name, format, cat) \
@@ -142,9 +129,8 @@ static struct attribute *mox_rwtm_attrs[] = { };ATTRIBUTE_GROUPS (mox_rwtm); -static void mox_kobj_release (struct kobject *kobj) +static void mox_kobj_release (struct kobject *) { -kfree (to_rwtm (kobj)->kobj); }static const struct kobj_type mox_kobj_ktype = {
@@ -155,18 +141,14 @@ static const struct kobj_type mox_kobj_ktype = {static int mox_kobj_create (struct mox_rwtm *rwtm) { - rwtm->kobj =kzalloc (sizeof (*rwtm->kobj),GFP_KERNEL ); -if (!rwtm->kobj) -return -ENOMEM ; +struct kobject *kobj = &rwtm->kobj; -kobject_init (rwtm_to_kobj (rwtm), &mox_kobj_ktype); -if (kobject_add (rwtm_to_kobj (rwtm), firmware_kobj,"turris-mox-rwtm" )) { -kobject_put (rwtm_to_kobj (rwtm)); +kobject_init (kobj, &mox_kobj_ktype); +if (kobject_add (kobj, firmware_kobj,"turris-mox-rwtm" )) { +kobject_put (kobj);return -ENXIO ; } - rwtm->kobj->rwtm = rwtm; -return 0 ; }
@@ -540,7 +522,7 @@ static int turris_mox_rwtm_probe(struct platform_device *pdev) free_channel:mbox_free_channel (rwtm->mbox); put_kobj: -kobject_put (rwtm_to_kobj (rwtm)); +kobject_put (&rwtm->kobj);return ret; }
@@ -548,7 +530,7 @@ static void turris_mox_rwtm_remove(struct platform_device *pdev) {struct mox_rwtm *rwtm =platform_get_drvdata (pdev); -kobject_put (rwtm_to_kobj (rwtm)); +kobject_put (&rwtm->kobj);mbox_free_channel (rwtm->mbox); }
Signature · 1 line
2.44.2
Marek Behún
maintainerPATCH 11/19 firmware: turris-mox-rwtm: Return true error code if kobject_add() fails
In reply to Marek BehúnReturn the error code from kobject_add() if it fails, instead of -ENXIO.
--- drivers/firmware/turris-mox-rwtm.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)
drivers/firmware/turris-mox-rwtm.c +5 -4
@@ -142,14 +142,15 @@ static const struct kobj_type mox_kobj_ktype = {static int mox_kobj_create (struct mox_rwtm *rwtm) {struct kobject *kobj = &rwtm->kobj; +int ret;kobject_init (kobj, &mox_kobj_ktype); -if (kobject_add (kobj, firmware_kobj,"turris-mox-rwtm" )) { + + ret =kobject_add (kobj, firmware_kobj,"turris-mox-rwtm" ); +if (ret)kobject_put (kobj); -return -ENXIO ; - } -return 0 ; +return ret; }static int mox_get_status (enum mbox_cmd cmd,u32 retval)
Signature · 1 line
2.44.2
Marek Behún
maintainerPATCH 12/19 firmware: turris-mox-rwtm: Convert rest to devm_* and get rid of driver .remove()
In reply to Marek BehúnUse devm resource management for driver's kobject and mailbox. This allows us to get rid of driver's .remove() method and gotos in .probe().
--- drivers/firmware/turris-mox-rwtm.c | 41 +++++++++++++++--------------- 1 file changed, 20 insertions(+), 21 deletions(-)
drivers/firmware/turris-mox-rwtm.c +20 -21
@@ -139,6 +139,11 @@ static const struct kobj_type mox_kobj_ktype = { .default_groups = mox_rwtm_groups, }; +static void mox_devm_kobj_release (void *kobj) +{ +kobject_put (kobj); +} +static int mox_kobj_create (struct mox_rwtm *rwtm) {struct kobject *kobj = &rwtm->kobj;
@@ -146,11 +151,11 @@ static int mox_kobj_create(struct mox_rwtm *rwtm)kobject_init (kobj, &mox_kobj_ktype); - ret =kobject_add (kobj, firmware_kobj,"turris-mox-rwtm" ); + ret =devm_add_action_or_reset (rwtm->dev, mox_devm_kobj_release, kobj);if (ret) -kobject_put (kobj); +return ret; -return ret; +return kobject_add (kobj, firmware_kobj,"turris-mox-rwtm" ); }static int mox_get_status (enum mbox_cmd cmd,u32 retval)
@@ -453,6 +458,11 @@ static inline void rwtm_register_debugfs(struct mox_rwtm *rwtm) }#endif +static void rwtm_devm_mbox_release (void *mbox) +{ +mbox_free_channel (mbox); +} +static int turris_mox_rwtm_probe (struct platform_device *pdev) {struct mox_rwtm *rwtm;
@@ -488,9 +498,13 @@ static int turris_mox_rwtm_probe(struct platform_device *pdev)if (ret != -EPROBE_DEFER )dev_err (dev,"Cannot request mailbox channel: %i\n" , ret); - goto put_kobj; +return ret; } + ret =devm_add_action_or_reset (dev, rwtm_devm_mbox_release, rwtm->mbox); +if (ret <0 ) +return ret; +init_completion (&rwtm->cmd_done); ret =mox_get_board_info (rwtm);
@@ -501,7 +515,7 @@ static int turris_mox_rwtm_probe(struct platform_device *pdev)if (ret <0 ) {dev_notice (dev,"Firmware does not support the GET_RANDOM command\n" ); - goto free_channel; +return ret; } rwtm->hwrng.name =DRIVER_NAME "_hwrng" ;
@@ -511,7 +525,7 @@ static int turris_mox_rwtm_probe(struct platform_device *pdev) ret =devm_hwrng_register (dev, &rwtm->hwrng);if (ret <0 ) {dev_err (dev,"Cannot register HWRNG: %i\n" , ret); - goto free_channel; +return ret; }rwtm_register_debugfs (rwtm);
@@ -519,20 +533,6 @@ static int turris_mox_rwtm_probe(struct platform_device *pdev)dev_info (dev,"HWRNG successfully registered\n" );return 0 ; - -free_channel: -mbox_free_channel (rwtm->mbox); -put_kobj: -kobject_put (&rwtm->kobj); -return ret; -} - -static void turris_mox_rwtm_remove (struct platform_device *pdev) -{ -struct mox_rwtm *rwtm =platform_get_drvdata (pdev); - -kobject_put (&rwtm->kobj); -mbox_free_channel (rwtm->mbox); }static const struct of_device_id turris_mox_rwtm_match[] = {
@@ -545,7 +545,6 @@ MODULE_DEVICE_TABLE(of, turris_mox_rwtm_match);static struct platform_driver turris_mox_rwtm_driver = { .probe = turris_mox_rwtm_probe, - .remove_new = turris_mox_rwtm_remove, .driver = { .name =DRIVER_NAME , .of_match_table = turris_mox_rwtm_match,
Signature · 1 line
2.44.2
Marek Behún
maintainerPATCH 13/19 firmware: turris-mox-rwtm: Use dev_err_probe() where possible
In reply to Marek BehúnUse dev_err_probe() where possible in the driver's .probe() method.
--- drivers/firmware/turris-mox-rwtm.c | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-)
drivers/firmware/turris-mox-rwtm.c +10 -12
@@ -480,10 +480,9 @@ static int turris_mox_rwtm_probe(struct platform_device *pdev)return -ENOMEM ; ret =mox_kobj_create (rwtm); -if (ret <0 ) { -dev_err (dev,"Cannot create turris-mox-rwtm kobject!\n" ); -return ret; - } +if (ret <0 ) +return dev_err_probe (dev, ret, +"Cannot create turris-mox-rwtm kobject!\n" );platform_set_drvdata (pdev, rwtm);
@@ -495,10 +494,11 @@ static int turris_mox_rwtm_probe(struct platform_device *pdev) rwtm->mbox =mbox_request_channel (&rwtm->mbox_client,0 );if (IS_ERR (rwtm->mbox)) { ret =PTR_ERR (rwtm->mbox); -if (ret != -EPROBE_DEFER ) -dev_err (dev,"Cannot request mailbox channel: %i\n" , - ret); -return ret; +if (ret == -EPROBE_DEFER ) +return ret; + +return dev_err_probe (dev, ret, +"Cannot request mailbox channel!\n" ); } ret =devm_add_action_or_reset (dev, rwtm_devm_mbox_release, rwtm->mbox);
@@ -523,10 +523,8 @@ static int turris_mox_rwtm_probe(struct platform_device *pdev) rwtm->hwrng.priv = (unsigned long ) rwtm; ret =devm_hwrng_register (dev, &rwtm->hwrng); -if (ret <0 ) { -dev_err (dev,"Cannot register HWRNG: %i\n" , ret); -return ret; - } +if (ret <0 ) +return dev_err_probe (dev, ret,"Cannot register HWRNG!\n" );rwtm_register_debugfs (rwtm);
Signature · 1 line
2.44.2
Marek Behún
maintainerPATCH 14/19 firmware: turris-mox-rwtm: Rearrange probe calls
In reply to Marek BehúnRearrange probe calls:
- create the kobject (and corresponding sysfs files) only after the mailbox is created and board info is read
- initialize the completion before mailbox channel is requested
--- drivers/firmware/turris-mox-rwtm.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-)
drivers/firmware/turris-mox-rwtm.c +6 -7
@@ -479,14 +479,10 @@ static int turris_mox_rwtm_probe(struct platform_device *pdev)if (!rwtm->buf)return -ENOMEM ; - ret =mox_kobj_create (rwtm); -if (ret <0 ) -return dev_err_probe (dev, ret, -"Cannot create turris-mox-rwtm kobject!\n" ); -platform_set_drvdata (pdev, rwtm);mutex_init (&rwtm->busy); +init_completion (&rwtm->cmd_done); rwtm->mbox_client.dev = dev; rwtm->mbox_client.rx_callback = mox_rwtm_rx_callback;
@@ -505,12 +501,15 @@ static int turris_mox_rwtm_probe(struct platform_device *pdev)if (ret <0 )return ret; -init_completion (&rwtm->cmd_done); - ret =mox_get_board_info (rwtm);if (ret <0 )dev_warn (dev,"Cannot read board information: %i\n" , ret); + ret =mox_kobj_create (rwtm); +if (ret <0 ) +return dev_err_probe (dev, ret, +"Cannot create turris-mox-rwtm kobject!\n" ); + ret =check_get_random_support (rwtm);if (ret <0 ) { dev_notice(dev,
Signature · 1 line
2.44.2
Marek Behún
maintainerPATCH 15/19 firmware: turris-mox-rwtm: Drop redundant device pointer
In reply to Marek BehúnDrop redundant device pointer from driver's private structure.
--- drivers/firmware/turris-mox-rwtm.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-)
drivers/firmware/turris-mox-rwtm.c +13 -8
@@ -58,7 +58,6 @@ enum mbox_cmd { };struct mox_rwtm { -struct device *dev;struct mbox_client mbox_client;struct mbox_chan *mbox;struct kobject kobj;
@@ -94,6 +93,11 @@ struct mox_rwtm {#endif }; +static inline struct device *rwtm_dev (struct mox_rwtm *rwtm) +{ +return rwtm->mbox_client.dev; +} +static inline struct mox_rwtm *to_rwtm (struct kobject *kobj) { return container_of(kobj, struct mox_rwtm, kobj);
@@ -151,7 +155,8 @@ static int mox_kobj_create(struct mox_rwtm *rwtm)kobject_init (kobj, &mox_kobj_ktype); - ret =devm_add_action_or_reset (rwtm->dev, mox_devm_kobj_release, kobj); + ret =devm_add_action_or_reset (rwtm_dev (rwtm), mox_devm_kobj_release, + kobj);if (ret)return ret;
@@ -196,6 +201,7 @@ static void reply_to_mac_addr(u8 *mac, u32 t1, u32 t2)static int mox_get_board_info (struct mox_rwtm *rwtm) { +struct device *dev =rwtm_dev (rwtm);struct armada_37xx_rwtm_tx_msg msg;struct armada_37xx_rwtm_rx_msg *reply = &rwtm->reply;int ret;
@@ -210,10 +216,10 @@ static int mox_get_board_info(struct mox_rwtm *rwtm) ret =mox_get_status (MBOX_CMD_BOARD_INFO , reply->retval);if (ret == -ENODATA ) { -dev_warn (rwtm->dev, +dev_warn (dev,"Board does not have manufacturing information burned!\n" ); }else if (ret == -ENOSYS ) { -dev_notice (rwtm->dev, +dev_notice (dev,"Firmware does not support the BOARD_INFO command\n" ); }else if (ret <0 ) {return ret;
@@ -245,9 +251,9 @@ static int mox_get_board_info(struct mox_rwtm *rwtm) ret =mox_get_status (MBOX_CMD_ECDSA_PUB_KEY , reply->retval);if (ret == -ENODATA ) { -dev_warn (rwtm->dev,"Board has no public key burned!\n" ); +dev_warn (dev,"Board has no public key burned!\n" ); }else if (ret == -ENOSYS ) { -dev_notice (rwtm->dev, +dev_notice (dev,"Firmware does not support the ECDSA_PUB_KEY command\n" ); }else if (ret <0 ) {return ret;
@@ -448,7 +454,7 @@ static void rwtm_register_debugfs(struct mox_rwtm *rwtm) root =debugfs_create_dir ("turris-mox-rwtm" ,NULL ); -devm_add_action_or_reset (rwtm->dev, rwtm_debugfs_release, root); +devm_add_action_or_reset (rwtm_dev (rwtm), rwtm_debugfs_release, root);debugfs_create_file_unsafe ("do_sign" ,0600 , root, rwtm, &do_sign_fops); }
@@ -473,7 +479,6 @@ static int turris_mox_rwtm_probe(struct platform_device *pdev)if (!rwtm)return -ENOMEM ; - rwtm->dev = dev; rwtm->buf =dmam_alloc_coherent (dev,PAGE_SIZE , &rwtm->buf_phys,GFP_KERNEL );if (!rwtm->buf)
Signature · 1 line
2.44.2
Marek Behún
maintainerPATCH 16/19 firmware: turris-mox-rwtm: Use devm_mutex_init() instead of mutex_init()
In reply to Marek BehúnUse devm_mutex_init() instead of mutex_init(), to properly call mutex_destroy() on probe failure / driver unbind.
--- drivers/firmware/turris-mox-rwtm.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
drivers/firmware/turris-mox-rwtm.c +4 -1
@@ -486,7 +486,10 @@ static int turris_mox_rwtm_probe(struct platform_device *pdev)platform_set_drvdata (pdev, rwtm); -mutex_init (&rwtm->busy); + ret =devm_mutex_init (dev, &rwtm->busy); +if (ret <0 ) +return ret; +init_completion (&rwtm->cmd_done); rwtm->mbox_client.dev = dev;
Signature · 1 line
2.44.2
Marek Behún
maintainerPATCH 17/19 firmware: turris-mox-rwtm: Use container_of() instead of hwrng .priv member
In reply to Marek BehúnUse container_of() to get driver private structure from hwnrg structure, instead of the hwrng's .priv member, as suggested by Herbert for another driver [1].
[1] https://lore.kernel.org/soc/[email protected]/
--- drivers/firmware/turris-mox-rwtm.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
drivers/firmware/turris-mox-rwtm.c +1 -2
@@ -292,7 +292,7 @@ static int check_get_random_support(struct mox_rwtm *rwtm)static int mox_hwrng_read (struct hwrng *rng,void *data,size_t max,bool wait) { -struct mox_rwtm *rwtm = (struct mox_rwtm *) rng->priv; + struct mox_rwtm *rwtm = container_of(rng, struct mox_rwtm, hwrng);struct armada_37xx_rwtm_tx_msg msg;int ret;
@@ -527,7 +527,6 @@ static int turris_mox_rwtm_probe(struct platform_device *pdev) rwtm->hwrng.name =DRIVER_NAME "_hwrng" ; rwtm->hwrng.read = mox_hwrng_read; - rwtm->hwrng.priv = (unsigned long ) rwtm; ret =devm_hwrng_register (dev, &rwtm->hwrng);if (ret <0 )
Signature · 1 line
2.44.2
Marek Behún
maintainerPATCH 18/19 firmware: turris-mox-rwtm: Deduplicate command execution code
In reply to Marek BehúnDeduplicate rWTM command execution calls
mbox_send_message()
wait_for_completion()
mox_get_status()
to one function
mox_rwtm_exec()
--- drivers/firmware/turris-mox-rwtm.c | 98 ++++++++++++------------------ 1 file changed, 39 insertions(+), 59 deletions(-)
drivers/firmware/turris-mox-rwtm.c +39 -59
@@ -189,6 +189,34 @@ static void mox_rwtm_rx_callback(struct mbox_client *cl, void *data)complete (&rwtm->cmd_done); } +static int mox_rwtm_exec (struct mox_rwtm *rwtm,enum mbox_cmd cmd, +struct armada_37xx_rwtm_tx_msg *msg, +bool interruptible) +{ +struct armada_37xx_rwtm_tx_msg _msg = {}; +int ret; + +if (!msg) + msg = &_msg; + + msg->command = cmd; + + ret =mbox_send_message (rwtm->mbox, msg); +if (ret <0 ) +return ret; + +if (interruptible) { + ret =wait_for_completion_interruptible (&rwtm->cmd_done); +if (ret <0 ) +return ret; + }else { +if (!wait_for_completion_timeout (&rwtm->cmd_done,HZ /2 )) +return -ETIMEDOUT ; + } + +return mox_get_status (cmd, rwtm->reply.retval); +} +static void reply_to_mac_addr (u8 *mac,u32 t1,u32 t2) { mac[0 ] = t1 >>8 ;
@@ -202,19 +230,10 @@ static void reply_to_mac_addr(u8 *mac, u32 t1, u32 t2)static int mox_get_board_info (struct mox_rwtm *rwtm) {struct device *dev =rwtm_dev (rwtm); -struct armada_37xx_rwtm_tx_msg msg;struct armada_37xx_rwtm_rx_msg *reply = &rwtm->reply;int ret; - msg.command =MBOX_CMD_BOARD_INFO ; - ret =mbox_send_message (rwtm->mbox, &msg); -if (ret <0 ) -return ret; - -if (!wait_for_completion_timeout (&rwtm->cmd_done,HZ /2 )) -return -ETIMEDOUT ; - - ret =mox_get_status (MBOX_CMD_BOARD_INFO , reply->retval); + ret =mox_rwtm_exec (rwtm,MBOX_CMD_BOARD_INFO ,NULL , false);if (ret == -ENODATA ) {dev_warn (dev,"Board does not have manufacturing information burned!\n" );
@@ -241,15 +260,7 @@ static int mox_get_board_info(struct mox_rwtm *rwtm)pr_info (" burned RAM size %i MiB\n" , rwtm->ram_size); } - msg.command =MBOX_CMD_ECDSA_PUB_KEY ; - ret =mbox_send_message (rwtm->mbox, &msg); -if (ret <0 ) -return ret; - -if (!wait_for_completion_timeout (&rwtm->cmd_done,HZ /2 )) -return -ETIMEDOUT ; - - ret =mox_get_status (MBOX_CMD_ECDSA_PUB_KEY , reply->retval); + ret =mox_rwtm_exec (rwtm,MBOX_CMD_ECDSA_PUB_KEY ,NULL , false);if (ret == -ENODATA ) {dev_warn (dev,"Board has no public key burned!\n" ); }else if (ret == -ENOSYS ) {
@@ -272,38 +283,24 @@ static int mox_get_board_info(struct mox_rwtm *rwtm)static int check_get_random_support (struct mox_rwtm *rwtm) { -struct armada_37xx_rwtm_tx_msg msg; -int ret; - - msg.command =MBOX_CMD_GET_RANDOM ; - msg.args[0 ] =1 ; - msg.args[1 ] = rwtm->buf_phys; - msg.args[2 ] =4 ; - - ret =mbox_send_message (rwtm->mbox, &msg); -if (ret <0 ) -return ret; - -if (!wait_for_completion_timeout (&rwtm->cmd_done,HZ /2 )) -return -ETIMEDOUT ; +struct armada_37xx_rwtm_tx_msg msg = { + .args = {1 , rwtm->buf_phys,4 }, + }; -return mox_get_status (MBOX_CMD_GET_RANDOM , rwtm->reply.retval); +return mox_rwtm_exec (rwtm,MBOX_CMD_GET_RANDOM , &msg, false); }static int mox_hwrng_read (struct hwrng *rng,void *data,size_t max,bool wait) { struct mox_rwtm *rwtm = container_of(rng, struct mox_rwtm, hwrng); -struct armada_37xx_rwtm_tx_msg msg; +struct armada_37xx_rwtm_tx_msg msg = { + .args = {1 , rwtm->buf_phys, (max +3 ) & ~3 }, + };int ret;if (max >PAGE_SIZE ) max =PAGE_SIZE ; - msg.command =MBOX_CMD_GET_RANDOM ; - msg.args[0 ] =1 ; - msg.args[1 ] = rwtm->buf_phys; - msg.args[2 ] = (max +3 ) & ~3 ; -if (!wait) {if (!mutex_trylock (&rwtm->busy))return -EBUSY ;
@@ -311,15 +308,7 @@ static int mox_hwrng_read(struct hwrng *rng, void *data, size_t max, bool wait)mutex_lock (&rwtm->busy); } - ret =mbox_send_message (rwtm->mbox, &msg); -if (ret <0 ) - goto unlock_mutex; - - ret =wait_for_completion_interruptible (&rwtm->cmd_done); -if (ret <0 ) - goto unlock_mutex; - - ret =mox_get_status (MBOX_CMD_GET_RANDOM , rwtm->reply.retval); + ret =mox_rwtm_exec (rwtm,MBOX_CMD_GET_RANDOM , &msg, true);if (ret <0 ) goto unlock_mutex;
@@ -367,7 +356,6 @@ static ssize_t do_sign_write(struct file *file, const char __user *buf, size_t len, loff_t *ppos) { struct mox_rwtm *rwtm = file->private_data; -struct armada_37xx_rwtm_rx_msg *reply = &rwtm->reply;struct armada_37xx_rwtm_tx_msg msg;loff_t dummy =0 ;ssize_t ret;
@@ -400,23 +388,15 @@ static ssize_t do_sign_write(struct file *file, const char __user *buf, goto unlock_mutex;be32_to_cpu_array (rwtm->buf, rwtm->buf,MOX_ECC_NUMBER_WORDS ); - msg.command =MBOX_CMD_SIGN ; msg.args[0 ] =1 ; msg.args[1 ] = rwtm->buf_phys; msg.args[2 ] = rwtm->buf_phys +MOX_ECC_NUMBER_LEN ; msg.args[3 ] = rwtm->buf_phys +2 *MOX_ECC_NUMBER_LEN ; - ret =mbox_send_message (rwtm->mbox, &msg); -if (ret <0 ) - goto unlock_mutex; - ret =wait_for_completion_interruptible (&rwtm->cmd_done); + ret =mox_rwtm_exec (rwtm,MBOX_CMD_SIGN , &msg, true);if (ret <0 ) goto unlock_mutex; - ret =MBOX_STS_VALUE (reply->retval); -if (MBOX_STS_ERROR (reply->retval) !=MBOX_STS_SUCCESS ) - goto unlock_mutex; -/* * Here we read the R and S values of the ECDSA signature * computed by the rWTM firmware and convert their words from
Signature · 1 line
2.44.2
Marek Behún
maintainerPATCH 19/19 firmware: turris-mox-rwtm: Use EOPNOTSUPP instead of ENOSYS
In reply to Marek BehúnCheckpatch warns agains -ENOSYS:
WARNING: ENOSYS means 'invalid syscall nr' and nothing else
Use EOPNOTSUPP instead.
--- drivers/firmware/turris-mox-rwtm.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/firmware/turris-mox-rwtm.c +3 -3
@@ -170,7 +170,7 @@ static int mox_get_status(enum mbox_cmd cmd, u32 retval)else if (MBOX_STS_ERROR (retval) ==MBOX_STS_FAIL )return -(int )MBOX_STS_VALUE (retval);else if (MBOX_STS_ERROR (retval) ==MBOX_STS_BADCMD ) -return -ENOSYS ; +return -EOPNOTSUPP ;else if (MBOX_STS_ERROR (retval) !=MBOX_STS_SUCCESS )return -EIO ;else
@@ -237,7 +237,7 @@ static int mox_get_board_info(struct mox_rwtm *rwtm)if (ret == -ENODATA ) {dev_warn (dev,"Board does not have manufacturing information burned!\n" ); - }else if (ret == -ENOSYS ) { + }else if (ret == -EOPNOTSUPP ) {dev_notice (dev,"Firmware does not support the BOARD_INFO command\n" ); }else if (ret <0 ) {
@@ -263,7 +263,7 @@ static int mox_get_board_info(struct mox_rwtm *rwtm) ret =mox_rwtm_exec (rwtm,MBOX_CMD_ECDSA_PUB_KEY ,NULL , false);if (ret == -ENODATA ) {dev_warn (dev,"Board has no public key burned!\n" ); - }else if (ret == -ENOSYS ) { + }else if (ret == -EOPNOTSUPP ) {dev_notice (dev,"Firmware does not support the ECDSA_PUB_KEY command\n" ); }else if (ret <0 ) {
Signature · 1 line
2.44.2
Re: [PATCH 03/19] firmware: turris-mox-rwtm: Use PAGE_SIZE instead of hardcoded 4096
In reply to Marek BehúnQuoted 14 lines · Marek Behún
The 4096 bytes limit in
mox_hwrng_read()is due to the DMA buffer being allocated to onePAGE_SIZEbytes. ThePAGE_SIZEmacro is used when allocating the buffer, use it inmox_hwrng_read()as well.Signed-off-by Marek Behún <[email protected]>--- drivers/firmware/turris-mox-rwtm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)drivers/firmware/turris-mox-rwtm.c +0
-0
drivers/firmware/turris-mox-rwtm.c static int mox_hwrng_read(struct hwrng *rng, void *data, size_t max, bool wait) · line 291
Quoted 7 lines
drivers/firmware/turris-mox-rwtm.c +2
-2@@ -287,8 +287,8 @@ static int mox_hwrng_read(struct hwrng *rng, void *data, size_t max, bool wait)struct armada_37xx_rwtm_tx_msg msg;int ret; -if (max >4096 ) - max =4096 ; +if (max >PAGE_SIZE ) + max =PAGE_SIZE ;
Wouldn't it be better to bind these to the alloc side with a local define that is set to PAGE_SIZE?
Signature · 1 line
i.
Re: [PATCH 04/19] firmware: turris-mox-rwtm: Use ETH_ALEN instead of hardcoded 6
In reply to Marek BehúnQuoted 20 lines · Marek Behún
Use the
ETH_ALENmacro instead of hardcoded 6 for MAC address length.Signed-off-by Marek Behún <[email protected]>--- drivers/firmware/turris-mox-rwtm.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)drivers/firmware/turris-mox-rwtm.c +1
-0@@ -10,6 +10,7 @@#include <linux/debugfs.h> #include <linux/dma-mapping.h> #include <linux/hw_random.h> +#include <linux/if_ether.h> #include <linux/mailbox_client.h> #include <linux/module.h> #include <linux/mutex.h>
drivers/firmware/turris-mox-rwtm.c struct mox_rwtm { · line 69
Quoted 5 lines
drivers/firmware/turris-mox-rwtm.c +1
-1@@ -65,7 +66,7 @@ struct mox_rwtm {int has_board_info;u64 serial_number;int board_version, ram_size; -u8 mac_address1[6 ], mac_address2[6 ]; +u8 mac_address1[ETH_ALEN ], mac_address2[ETH_ALEN ];
Signature · 1 line
i.
Re: [PATCH 02/19] firmware: turris-mox-rwtm: Fix checking return value of wait_for_completion_timeout()
In reply to Marek BehúnQuoted 39 lines · Marek Behún
The
wait_for_completion_timeout()function returns 0 if timed out, and a positive value if completed. Fix the usage of this function.Fixes 389711b37493 ("firmware: Add Turris Mox rWTM firmware driver")Fixes 2eab59cf0d20 ("firmware: turris-mox-rwtm: fail probing when firmware does not support hwrng")Signed-off-by Marek Behún <[email protected]>--- drivers/firmware/turris-mox-rwtm.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-)drivers/firmware/turris-mox-rwtm.c +4
-6@@ -202,9 +202,8 @@ static int mox_get_board_info(struct mox_rwtm *rwtm)if (ret <0 )return ret; - ret =wait_for_completion_timeout (&rwtm->cmd_done,HZ /2 ); -if (ret <0 ) -return ret; +if (!wait_for_completion_timeout (&rwtm->cmd_done,HZ /2 )) +return -ETIMEDOUT ; ret =mox_get_status (MBOX_CMD_BOARD_INFO , reply->retval);if (ret == -ENODATA ) {@@ -238,9 +237,8 @@ static int mox_get_board_info(struct mox_rwtm *rwtm)if (ret <0 )return ret; - ret =wait_for_completion_timeout (&rwtm->cmd_done,HZ /2 ); -if (ret <0 ) -return ret; +if (!wait_for_completion_timeout (&rwtm->cmd_done,HZ /2 )) +return -ETIMEDOUT ; ret =mox_get_status (MBOX_CMD_ECDSA_PUB_KEY , reply->retval);if (ret == -ENODATA ) {
drivers/firmware/turris-mox-rwtm.c static int check_get_random_support(struct mox_rwtm *rwtm) · line 279
Quoted 8 lines
drivers/firmware/turris-mox-rwtm.c +2
-3@@ -277,9 +275,8 @@ static int check_get_random_support(struct mox_rwtm *rwtm)if (ret <0 )return ret; - ret =wait_for_completion_timeout (&rwtm->cmd_done,HZ /2 ); -if (ret <0 ) -return ret; +if (!wait_for_completion_timeout (&rwtm->cmd_done,HZ /2 )) +return -ETIMEDOUT ;
Signature · 1 line
i.
Re: [PATCH 08/19] firmware: turris-mox-rwtm: Use kobj_type's default_groups member for automatic sysfs files creation
In reply to Marek BehúnQuoted 15 lines · Marek Behún
Put the turris-mox-rwtm attribute group into the .default_groups member of the underlying kobject type and drop the manual
sysfs_create_files()/sysfs_remove_files(). The kobject library will take care of this in it's internal code.Signed-off-by Marek Behún <[email protected]>--- drivers/firmware/turris-mox-rwtm.c | 71 ++++++++++++++---------------- 1 file changed, 32 insertions(+), 39 deletions(-)drivers/firmware/turris-mox-rwtm.c +0
-0
drivers/firmware/turris-mox-rwtm.c static inline struct mox_rwtm *to_rwtm(struct kobject *kobj) · line 121
Quoted 9 lines
drivers/firmware/turris-mox-rwtm.c +6
-0@@ -113,6 +113,36 @@ static inline struct mox_rwtm *to_rwtm(struct kobject *kobj) return container_of(kobj, struct mox_kobject, kobj)->rwtm; } +#define MOX_ATTR_RO (name, format, cat) \ +static ssize_t \ +name##_show(struct kobject *kobj, struct kobj_attribute *a, \ + char *buf) \ +{ \ + struct mox_rwtm *rwtm = to_rwtm(kobj); \
Fix \ alignment to match the others while you move the code around.
Signature · 1 line
i.
Quoted 35 lines
drivers/firmware/turris-mox-rwtm.c +25
-0+ if (!rwtm->has_##cat) \ + return -ENODATA; \ + return sprintf(buf, format, rwtm->name); \ +} \ +static struct kobj_attribute mox_attr_##name = __ATTR_RO(name) + +MOX_ATTR_RO (serial_number,"%016llX\n" , board_info); +MOX_ATTR_RO (board_version,"%i\n" , board_info); +MOX_ATTR_RO (ram_size,"%i\n" , board_info); +MOX_ATTR_RO (mac_address1,"%pM\n" , board_info); +MOX_ATTR_RO (mac_address2,"%pM\n" , board_info); +MOX_ATTR_RO (pubkey,"%s\n" , pubkey); + +static struct attribute *mox_rwtm_attrs[] = { + &mox_attr_serial_number.attr, + &mox_attr_board_version.attr, + &mox_attr_ram_size.attr, + &mox_attr_mac_address1.attr, + &mox_attr_mac_address2.attr, + &mox_attr_pubkey.attr, +NULL +}; +ATTRIBUTE_GROUPS (mox_rwtm); +static void mox_kobj_release (struct kobject *kobj) {kfree (to_rwtm (kobj)->kobj);@@ -121,6 +151,7 @@ static void mox_kobj_release(struct kobject *kobj)static const struct kobj_type mox_kobj_ktype = { .release = mox_kobj_release, .sysfs_ops = &kobj_sysfs_ops, + .default_groups = mox_rwtm_groups, };static int mox_kobj_create (struct mox_rwtm *rwtm)
Re: [PATCH 10/19] firmware: turris-mox-rwtm: Simplify driver kobject code
In reply to Marek BehúnQuoted 58 lines · Marek Behún
Drop the
mox_kobjectwrapper that needs to be allocated, instead put the kobject directly into the driver private structure. This allows us to drop onekzalloc()call.Signed-off-by Marek Behún <[email protected]>--- drivers/firmware/turris-mox-rwtm.c | 36 ++++++++---------------------- 1 file changed, 9 insertions(+), 27 deletions(-)drivers/firmware/turris-mox-rwtm.c +2
-15@@ -21,7 +21,6 @@#include <linux/module.h> #include <linux/mutex.h> #include <linux/platform_device.h> -#include <linux/slab.h> #include <linux/sysfs.h> #include <linux/types.h> @@ -58,13 +57,11 @@ enum mbox_cmd { MBOX_CMD_OTP_WRITE = 8, }; -struct mox_kobject ; -struct mox_rwtm {struct device *dev;struct mbox_client mbox_client;struct mbox_chan *mbox; -struct mox_kobject *kobj; +struct kobject kobj;struct hwrng hwrng;struct armada_37xx_rwtm_rx_msg reply;@@ -97,19 +94,9 @@ struct mox_rwtm {#endif }; -struct mox_kobject { -struct kobject kobj; -struct mox_rwtm *rwtm; -}; - -static inline struct kobject *rwtm_to_kobj (struct mox_rwtm *rwtm) -{ -return &rwtm->kobj->kobj; -} -static inline struct mox_rwtm *to_rwtm (struct kobject *kobj) { - return container_of(kobj, struct mox_kobject, kobj)->rwtm; + return container_of(kobj, struct mox_rwtm, kobj); }#define MOX_ATTR_RO (name, format, cat) \
drivers/firmware/turris-mox-rwtm.c static struct attribute *mox_rwtm_attrs[] = { · line 134
Quoted 8 lines
drivers/firmware/turris-mox-rwtm.c +1
-2@@ -142,9 +129,8 @@ static struct attribute *mox_rwtm_attrs[] = { };ATTRIBUTE_GROUPS (mox_rwtm); -static void mox_kobj_release (struct kobject *kobj) +static void mox_kobj_release (struct kobject *) { -kfree (to_rwtm (kobj)->kobj); }
Is empty .release necessary at all? I found some kobj_type structs without .release so I'd expect it to be unnecessary.
Signature · 1 line
i.
Quoted 43 lines
drivers/firmware/turris-mox-rwtm.c +6
-10static const struct kobj_type mox_kobj_ktype = {@@ -155,18 +141,14 @@ static const struct kobj_type mox_kobj_ktype = {static int mox_kobj_create (struct mox_rwtm *rwtm) { - rwtm->kobj =kzalloc (sizeof (*rwtm->kobj),GFP_KERNEL ); -if (!rwtm->kobj) -return -ENOMEM ; +struct kobject *kobj = &rwtm->kobj; -kobject_init (rwtm_to_kobj (rwtm), &mox_kobj_ktype); -if (kobject_add (rwtm_to_kobj (rwtm), firmware_kobj,"turris-mox-rwtm" )) { -kobject_put (rwtm_to_kobj (rwtm)); +kobject_init (kobj, &mox_kobj_ktype); +if (kobject_add (kobj, firmware_kobj,"turris-mox-rwtm" )) { +kobject_put (kobj);return -ENXIO ; } - rwtm->kobj->rwtm = rwtm; -return 0 ; }@@ -540,7 +522,7 @@ static int turris_mox_rwtm_probe(struct platform_device *pdev) free_channel:mbox_free_channel (rwtm->mbox); put_kobj: -kobject_put (rwtm_to_kobj (rwtm)); +kobject_put (&rwtm->kobj);return ret; }@@ -548,7 +530,7 @@ static void turris_mox_rwtm_remove(struct platform_device *pdev) {struct mox_rwtm *rwtm =platform_get_drvdata (pdev); -kobject_put (rwtm_to_kobj (rwtm)); +kobject_put (&rwtm->kobj);mbox_free_channel (rwtm->mbox); }
Marek Behún
maintainerRe: [PATCH 03/19] firmware: turris-mox-rwtm: Use PAGE_SIZE instead of hardcoded 4096
In reply to Ilpo JärvinenQuoted 19 lines · On Thu, 13 Jun 2024 11:01:42 +0300 (EEST) Ilpo Järvinen…
Quoted 17 lines · Marek Behún
The 4096 bytes limit in
mox_hwrng_read()is due to the DMA buffer being allocated to onePAGE_SIZEbytes. ThePAGE_SIZEmacro is used when allocating the buffer, use it inmox_hwrng_read()as well.Signed-off-by Marek Behún <[email protected]>--- drivers/firmware/turris-mox-rwtm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)drivers/firmware/turris-mox-rwtm.c +0
-0@@ -287,8 +287,8 @@ static int mox_hwrng_read(struct hwrng *rng, void *data, size_t max, bool wait)struct armada_37xx_rwtm_tx_msg msg;int ret;
Quoted 7 lines
drivers/firmware/turris-mox-rwtm.c static int mox_hwrng_read(struct hwrng *rng, void *data, size_t max, bool wait) · line 291
Quoted 4 lines
drivers/firmware/turris-mox-rwtm.c +2
-2-if (max >4096 ) - max =4096 ; +if (max >PAGE_SIZE ) + max =PAGE_SIZE ;Wouldn't it be better to bind these to the alloc side with a local define that is set to
PAGE_SIZE?
OK.
Marek Behún
maintainerRe: [PATCH 10/19] firmware: turris-mox-rwtm: Simplify driver kobject code
In reply to Ilpo JärvinenQuoted 2 lines · On Thu, 13 Jun 2024 11:31:43 +0300 (EEST) Ilpo Järvinen…
Is empty .release necessary at all? I found some
kobj_typestructs without .release so I'd expect it to be unnecessary.
lib/kobject.c function kobject_cleanup() has the following:
if (t && !t->release)
pr_debug("'%s' (%p): does not have a release() function, it is broken and must be fixed. See Documentation/core-api/kobject.rst.\n",
Marek
Re: [PATCH 10/19] firmware: turris-mox-rwtm: Simplify driver kobject code
Quoted 10 lines · Marek Behún
Quoted 2 lines · On Thu, 13 Jun 2024 11:31:43 +0300 (EEST) Ilpo Järvinen…
Is empty .release necessary at all? I found some
kobj_typestructs without .release so I'd expect it to be unnecessary.
lib/kobject.cfunctionkobject_cleanup()has the following:if (t && !t->release)
pr_debug("'%s' (%p): does not have arelease()function, it is broken and must be fixed. See Documentation/core-api/kobject.rst.\n",
Hmm, the plot thickens... that documentation file says:
'Do not try to get rid of this warning by providing an "empty" release function.'
?
Signature · 1 line
i.
Marek Behún
maintainerRe: [PATCH 10/19] firmware: turris-mox-rwtm: Simplify driver kobject code
Quoted 13 lines · On Thu, 13 Jun 2024 13:19:47 +0300 (EEST) Ilpo Järvinen…
Quoted 10 lines · Marek Behún
Quoted 2 lines · On Thu, 13 Jun 2024 11:31:43 +0300 (EEST) Ilpo Järvinen…
Is empty .release necessary at all? I found some
kobj_typestructs without .release so I'd expect it to be unnecessary.
lib/kobject.cfunctionkobject_cleanup()has the following:if (t && !t->release)
pr_debug("'%s' (%p): does not have arelease()function, it is broken and must be fixed. See Documentation/core-api/kobject.rst.\n",
Quoted 6 lines
Hmm, the plot thickens... that documentation file says:
'Do not try to get rid of this warning by providing an "empty" release function.'
?
This whole thing stinks. I will rewrite it so that the attributes will be under the device's kobject, as they should be. This way I can get rid of the whole own kobject type.
Then I will add a symlink from /sys/firmware/turris-mox-rwtm to the device, for sysfs ABI compatibility.
Marek
Re: [PATCH 10/19] firmware: turris-mox-rwtm: Simplify driver kobject code
Quoted 18 lines · Marek Behún
Quoted 15 lines · On Thu, 13 Jun 2024 13:19:47 +0300 (EEST) Ilpo Järvinen…
Quoted 10 lines · Marek Behún
Quoted 2 lines · On Thu, 13 Jun 2024 11:31:43 +0300 (EEST) Ilpo Järvinen…
Is empty .release necessary at all? I found some
kobj_typestructs without .release so I'd expect it to be unnecessary.
lib/kobject.cfunctionkobject_cleanup()has the following:if (t && !t->release)
pr_debug("'%s' (%p): does not have arelease()function, it is broken and must be fixed. See Documentation/core-api/kobject.rst.\n",Hmm, the plot thickens... that documentation file says:
Quoted 8 lines
Quoted 4 lines
'Do not try to get rid of this warning by providing an "empty" release function.'
?
This whole thing stinks. I will rewrite it so that the attributes will be under the device's kobject, as they should be. This way I can get rid of the whole own kobject type.
Yeah, I didn't really understand why they weren't there in the first place.
Quoted 2 lines
Then I will add a symlink from
/sys/firmware/turris-mox-rwtmto the device, for sysfs ABI compatibility.
BTW (unrelated to any of your patches unless I missed something)... You might want to check one unlock_mutex: label that seemed to be 100% duplicate the tail of the return path.
Signature · 1 line
i.
Marek Behún
maintainerRe: [PATCH 10/19] firmware: turris-mox-rwtm: Simplify driver kobject code
Quoted 32 lines · On Thu, 13 Jun 2024 16:37:23 +0300 (EEST) Ilpo Järvinen…
Quoted 18 lines · Marek Behún
Quoted 15 lines · On Thu, 13 Jun 2024 13:19:47 +0300 (EEST) Ilpo Järvinen…
Quoted 10 lines · Marek Behún
Quoted 2 lines · On Thu, 13 Jun 2024 11:31:43 +0300 (EEST) Ilpo Järvinen…
Is empty .release necessary at all? I found some
kobj_typestructs without .release so I'd expect it to be unnecessary.
lib/kobject.cfunctionkobject_cleanup()has the following:if (t && !t->release)
pr_debug("'%s' (%p): does not have arelease()function, it is broken and must be fixed. See Documentation/core-api/kobject.rst.\n",Hmm, the plot thickens... that documentation file says:
Quoted 8 lines
Quoted 4 lines
'Do not try to get rid of this warning by providing an "empty" release function.'
?
This whole thing stinks. I will rewrite it so that the attributes will be under the device's kobject, as they should be. This way I can get rid of the whole own kobject type.
Yeah, I didn't really understand why they weren't there in the first place.
Quoted 7 lines
Quoted 2 lines
Then I will add a symlink from
/sys/firmware/turris-mox-rwtmto the device, for sysfs ABI compatibility.BTW (unrelated to any of your patches unless I missed something)... You might want to check one
unlock_mutex: label that seemed to be 100% duplicate the tail of the return path.
Do you mean this?
mutex_unlock(&rwtm->busy);
return len;
unlock_mutex:
mutex_unlock(&rwtm->busy);
return ret;It's not 100% duplicate, but yes, I could do ret = len.
The thing is that this whole debugfs code is going away. I wanted to clean the driver up a little before converting this debugfs code to keyctl (which is the correct API for the thing that is now exposed to userspace via debugfs).
Marek
Re: [PATCH 10/19] firmware: turris-mox-rwtm: Simplify driver kobject code
Quoted 45 lines · Marek Behún
Quoted 32 lines · On Thu, 13 Jun 2024 16:37:23 +0300 (EEST) Ilpo Järvinen…
Quoted 18 lines · Marek Behún
Quoted 15 lines · On Thu, 13 Jun 2024 13:19:47 +0300 (EEST) Ilpo Järvinen…
Quoted 10 lines · Marek Behún
Quoted 2 lines · On Thu, 13 Jun 2024 11:31:43 +0300 (EEST) Ilpo Järvinen…
Is empty .release necessary at all? I found some
kobj_typestructs without .release so I'd expect it to be unnecessary.
lib/kobject.cfunctionkobject_cleanup()has the following:if (t && !t->release)
pr_debug("'%s' (%p): does not have arelease()function, it is broken and must be fixed. See Documentation/core-api/kobject.rst.\n",Hmm, the plot thickens... that documentation file says:
Quoted 8 lines
Quoted 4 lines
'Do not try to get rid of this warning by providing an "empty" release function.'
?
This whole thing stinks. I will rewrite it so that the attributes will be under the device's kobject, as they should be. This way I can get rid of the whole own kobject type.
Yeah, I didn't really understand why they weren't there in the first place.
Quoted 7 lines
Quoted 2 lines
Then I will add a symlink from
/sys/firmware/turris-mox-rwtmto the device, for sysfs ABI compatibility.BTW (unrelated to any of your patches unless I missed something)... You might want to check one
unlock_mutex: label that seemed to be 100% duplicate the tail of the return path.Do you mean this?
Quoted 7 lines
mutex_unlock(&rwtm->busy); return len; unlock_mutex: mutex_unlock(&rwtm->busy); return ret;It's not 100% duplicate, but yes, I could do ret = len.
Ah sorry, I didn't realize the variable name was different.
Quoted 4 lines
The thing is that this whole debugfs code is going away. I wanted to clean the driver up a little before converting this debugfs code to keyctl (which is the correct API for the thing that is now exposed to userspace via debugfs).
Signature · 1 line
i.