| Message ID | 2d432e67cab2eb51f36f5b2e904a185ef48df6e0.1579423564.git.lijiazi@xiaomi.com |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | None | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | warning | "total: 0 errors, 1 warnings, 13 lines checked" |
| robh/checkpatch | warning | "total: 0 errors, 1 warnings, 13 lines checked" |
On (20/01/20 19:38), lijiazi wrote: [..] > Changes in v3: > - fix incorrect multi-line comment style. > - split v2 to 3 patches. > --- > include/linux/of.h | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/include/linux/of.h b/include/linux/of.h > index c669c0a..c5bbfa6 100644 > --- a/include/linux/of.h > +++ b/include/linux/of.h > @@ -147,6 +147,13 @@ extern raw_spinlock_t devtree_lock; > #define OF_OVERLAY 5 /* allocated for an overlay */ > #define OF_OVERLAY_FREE_CSET 6 /* in overlay cset being freed */ > > +/* > + * add OF_DEVICE_NODE_FLAG_MAX for %pOF related printk. > + * if there is any change on these flags, please synchronize the change > + * to device_node_string function in lib/vsprintf.c. So maybe the flags can become enum then? > + */ > +#define OF_DEVICE_NODE_FLAG_MAX 6 /* Maximum available flags */ -ss
On Mon, Jan 20, 2020 at 07:38:28PM +0800, lijiazi wrote: > introduce OF_DEVICE_NODE_FLAG_MAX for %pOF related printk. introduce -> Introduce > @@ -147,6 +147,13 @@ extern raw_spinlock_t devtree_lock; > #define OF_OVERLAY 5 /* allocated for an overlay */ > #define OF_OVERLAY_FREE_CSET 6 /* in overlay cset being freed */ > +/* > + * add OF_DEVICE_NODE_FLAG_MAX for %pOF related printk. > + * if there is any change on these flags, please synchronize the change > + * to device_node_string function in lib/vsprintf.c. This is doubtful comment. If we don't want to print some flags in printf()? And in general this is orthogonal to the printf() changes. > + */ > +#define OF_DEVICE_NODE_FLAG_MAX 6 /* Maximum available flags */
On Mon, Jan 20, 2020 at 07:38:29PM +0800, lijiazi wrote: > Add two device node flags, and use OF_DEVICE_NODE_FLAG_MAX instead > of sizeof("xxxx"). ... > tbuf[1] = of_node_check_flag(dn, OF_DETACHED) ? 'd' : '-'; > tbuf[2] = of_node_check_flag(dn, OF_POPULATED) ? 'P' : '-'; > tbuf[3] = of_node_check_flag(dn, OF_POPULATED_BUS) ? 'B' : '-'; > - tbuf[4] = 0; This is fine to leave untouched. See below. > + tbuf[4] = of_node_check_flag(dn, OF_OVERLAY) ? 'O' : '-'; > + tbuf[5] = of_node_check_flag(dn, OF_OVERLAY_FREE_CSET) ? 'F' : '-'; These two should be part of patch 1, which in turn should be last in the series. > + tbuf[OF_DEVICE_NODE_FLAG_MAX] = 0; This one also, but in a form of explicit number, if you afraid of problems here, we may add something like BUILD_BUG_ON(OF_DEVICE_NODE_FLAG_MAX < ...); where ... depends on amount of flags we print here.
On Mon 2020-01-20 16:19:51, Andy Shevchenko wrote: > On Mon, Jan 20, 2020 at 07:38:29PM +0800, lijiazi wrote: > > Add two device node flags, and use OF_DEVICE_NODE_FLAG_MAX instead > > of sizeof("xxxx"). > > ... > > > tbuf[1] = of_node_check_flag(dn, OF_DETACHED) ? 'd' : '-'; > > tbuf[2] = of_node_check_flag(dn, OF_POPULATED) ? 'P' : '-'; > > tbuf[3] = of_node_check_flag(dn, OF_POPULATED_BUS) ? 'B' : '-'; > > > - tbuf[4] = 0; > > This is fine to leave untouched. See below. > > > + tbuf[4] = of_node_check_flag(dn, OF_OVERLAY) ? 'O' : '-'; > > + tbuf[5] = of_node_check_flag(dn, OF_OVERLAY_FREE_CSET) ? 'F' : '-'; > > These two should be part of patch 1, which in turn should be last in the series. > > > + tbuf[OF_DEVICE_NODE_FLAG_MAX] = 0; > > This one also, but in a form of explicit number, if you afraid of problems > here, we may add something like I agree that explicit number would be better here: tbuf[6] = 0; And I like the build check. I would even check for the exact number. It would help to keep the code updated when the number is modified. BUILD_BUG_ON(OF_DEVICE_NODE_FLAG_MAX != 6); Finally, I would put all three changes into the same patch. And remove the comment above OF_DEVICE_NODE_FLAG_MAX definition completely. Best Regards, Petr
diff --git a/include/linux/of.h b/include/linux/of.h index c669c0a..c5bbfa6 100644 --- a/include/linux/of.h +++ b/include/linux/of.h @@ -147,6 +147,13 @@ extern raw_spinlock_t devtree_lock; #define OF_OVERLAY 5 /* allocated for an overlay */ #define OF_OVERLAY_FREE_CSET 6 /* in overlay cset being freed */ +/* + * add OF_DEVICE_NODE_FLAG_MAX for %pOF related printk. + * if there is any change on these flags, please synchronize the change + * to device_node_string function in lib/vsprintf.c. + */ +#define OF_DEVICE_NODE_FLAG_MAX 6 /* Maximum available flags */ + #define OF_BAD_ADDR ((u64)-1) #ifdef CONFIG_OF
introduce OF_DEVICE_NODE_FLAG_MAX for %pOF related printk. Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Signed-off-by: lijiazi <lijiazi@xiaomi.com> --- Changes in v3: - fix incorrect multi-line comment style. - split v2 to 3 patches. --- include/linux/of.h | 7 +++++++ 1 file changed, 7 insertions(+)