On Tue, 02 Aug 2016, Baole Ni wrote: > I find that the developers often just specified the numeric value > when calling a macro which is defined with a parameter for access permission. > As we know, these numeric value for access permission have had the > corresponding macro, > and that using macro can improve the robustness and readability of the code, > thus, I suggest replacing the numeric parameter with the macro. > > Signed-off-by: Chuansheng Liu <[email protected]> > Signed-off-by: Baole Ni <[email protected]> > --- > drivers/mfd/sm501.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-)
I can tell you now that this patch-set is going to cause some upset. First and foremost, the change doesn't necessarily improve anything. Many of us actually prefer the numeric representation for file permissions -- I know that I do! The description is wrong too, since they are simple defines, not macros. More importantly, you're breaking many of our official (and unwritten) submitting patches rules. For instance, this is taken directly from Documentation/SubmittingPatches: "Do not send more than 15 patches at once to the vger mailing lists!!!" Arguably, you have been too aggressive with your Cc: list. People who only made simple changes to a file X months ago, do not need to receive/review your patch, and certainly don't expect to have their inbox filled by someone who wasn't thinking clearly or using a script to spam kernel contributors. The output of get_maintainers.pl should be taken objectively and a certain amount of common sense applied. By writing and submitting your patch-set using a script, you've removed the possibility of doing that. IMO, the patch-set is too granular. If I were do make a one line change in many files across the kernel, I would have probably done so one subsystem at a time, or at the very least, one patch per subsystem. The only reason for submitting using your chosen method would be to artificially increase your upstream creds (patch count). Unfortunately, by submitting this way, I fear this may have achieved this exact opposite. I don't seem to be able to locate a 0th patch. This is normally submitted to everyone, so if any person wished to share their views with the remainder of the recipients (which I'm sure many do), they would do so there. It also provides the full-diff, which is very useful when debugging and an opportunity to offer your reasoning for any unusual patch submitting behaviour. This too would have been useful. Anyway, as you're probably already aware, for the MFD and Backlight patches, it's a NACK I'm afraid. > diff --git a/drivers/mfd/sm501.c b/drivers/mfd/sm501.c > index 65cd0d2..6837aa3 100644 > --- a/drivers/mfd/sm501.c > +++ b/drivers/mfd/sm501.c > @@ -1216,7 +1216,7 @@ static ssize_t sm501_dbg_regs(struct device *dev, > } > > > -static DEVICE_ATTR(dbg_regs, 0444, sm501_dbg_regs, NULL); > +static DEVICE_ATTR(dbg_regs, S_IRUSR | S_IRGRP | S_IROTH, sm501_dbg_regs, > NULL); > > /* sm501_init_reg > * -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog
