| Message ID | 20171109002741.10897-1-afd@ti.com |
|---|---|
| Headers | show |
| Series | Add Headphone Detection to TLV320AIC31xx Driver | expand |
On Wed, Nov 08, 2017 at 06:27:25PM -0600, Andrew F. Davis wrote: > Simple non-functional changes including: > > * Fix header copyright tags > * Fix spelling errors > * Reformat code for easier reading > * Move some code blocks to a more natural ordering > * Remove unneeded code > * Remove assignments that are always overridden > * Normalize function return paths > > Signed-off-by: Andrew F. Davis <afd@ti.com> There's other things in here like adding error reporting... please don't send changes like this, if you want to do cleanups you should split them up in the same way you would other changes. Bigger patches are harder to review especially if they're not repetitive examples of the same pattern.
On Wed, Nov 08, 2017 at 06:27:27PM -0600, Andrew F. Davis wrote: > GPIO1 control register is number 51, fix this here. > > Fixes: bafcbfe429eb ("ASoC: tlv320aic31xx: Make the register values human readable") > Signed-off-by: Andrew F. Davis <afd@ti.com> > --- We should send this as a bug fix to stable but this depends on your reformatting :(
On 11/09/2017 06:41 AM, Mark Brown wrote: > On Wed, Nov 08, 2017 at 06:27:25PM -0600, Andrew F. Davis wrote: >> Simple non-functional changes including: >> >> * Fix header copyright tags >> * Fix spelling errors >> * Reformat code for easier reading >> * Move some code blocks to a more natural ordering >> * Remove unneeded code >> * Remove assignments that are always overridden >> * Normalize function return paths >> >> Signed-off-by: Andrew F. Davis <afd@ti.com> > > There's other things in here like adding error reporting... please > don't send changes like this, if you want to do cleanups you should > split them up in the same way you would other changes. Bigger patches > are harder to review especially if they're not repetitive examples of > the same pattern. > I'm never really sure with these where the split point should be, almost every change in here could be its own patch if I really wanted to pad my kernel patch count, but this series is already 17 patches long and I usually see these all as the same logical action: non-functional cleanups. I agree the added error message isn't purely non-functional and so should be broken out, I'll break out a couple other changes for v2. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 11/09/2017 06:45 AM, Mark Brown wrote: > On Wed, Nov 08, 2017 at 06:27:27PM -0600, Andrew F. Davis wrote: >> GPIO1 control register is number 51, fix this here. >> >> Fixes: bafcbfe429eb ("ASoC: tlv320aic31xx: Make the register values human readable") >> Signed-off-by: Andrew F. Davis <afd@ti.com> >> --- > > We should send this as a bug fix to stable but this depends on your > reformatting :( > I put it after the reformatting as an example of its value, now that you have seen it, I'll move it before the reformatting so it can be taken independently. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Nov 09, 2017 at 08:13:17AM -0600, Andrew F. Davis wrote: > On 11/09/2017 06:41 AM, Mark Brown wrote: > > There's other things in here like adding error reporting... please > > don't send changes like this, if you want to do cleanups you should > > split them up in the same way you would other changes. Bigger patches > > are harder to review especially if they're not repetitive examples of > > the same pattern. > I'm never really sure with these where the split point should be, almost > every change in here could be its own patch if I really wanted to pad my > kernel patch count, but this series is already 17 patches long and I > usually see these all as the same logical action: non-functional cleanups. > I agree the added error message isn't purely non-functional and so > should be broken out, I'll break out a couple other changes for v2. This is why cleanups don't happen :) A separate cleanup series would probably cover it, when they're split up well they're quick and easy to review.