| Message ID | Y3u0Oydiv2Wauda2@spud (mailing list archive) |
|---|---|
| State | Queued |
| Headers | show |
| Series | RISC-V SoC Drivers for v6.2 | expand |
On Mon, Nov 21, 2022, at 18:24, Conor Dooley wrote: > Hey Arnd, > > Same stuff applies here: lmk if there's something you'd rather see changed. > Perhaps you'd prefer to see PRs per vendor? Although I think that's less > likely to matter here than in the DT stuff. Again, I'll try to get the PR > out a bit earlier next time. Applied, this looks fine, just a few things to keep in mind: - please add "[GIT PULL]" to the subject line of the email - Splitting up a large pull request into smaller ones can be helpful to make sure things don't go in unnoticed. I try to (briefly) look at each patch, but if you have 20 boring but large patches, and a small but important patch that I may need to comment on, that is a good reason to split. > Not too much to see here, Yang Yingliang has added some error handling > to the setup of the driver that reports SiFive cache topology > information. I've put it on -next given how far we are in the release > cycle, feel free to put it on fixes if you disagree :) This is fine either way, as none of the fixes are likely to cause any real issues. I usually like to err on the side of having too much in the fixes branch instead of risking to miss something, but I'm just as happy to follow your preference here. > RISC-V SoC drivers for v6.2 > > SiFive: > - add probe error handling to the ccache driver Since this tag description becomes part of the git history, try to write it like you would write a commit log in the future. Ideally that avoids bulleted lists (I know they are easy) and instead uses full sentences that explain things about the state of the patches. If there are bugfixes, are users likely to need the fixes or were they found through inspection? For new features, explain who would have the corresponding hardware and what it does. Again, what you have here is not wrong, but it can always get better. Arnd
On Tue, 22 Nov 2022 06:38:40 PST (-0800), Arnd Bergmann wrote: > On Mon, Nov 21, 2022, at 18:24, Conor Dooley wrote: >> Hey Arnd, >> >> Same stuff applies here: lmk if there's something you'd rather see changed. >> Perhaps you'd prefer to see PRs per vendor? Although I think that's less >> likely to matter here than in the DT stuff. Again, I'll try to get the PR >> out a bit earlier next time. > > Applied, this looks fine, just a few things to keep in mind: Thanks! > - please add "[GIT PULL]" to the subject line of the email FWIW, here's the script I use to send pull requests: https://github.com/palmer-dabbelt/home/blob/master/.local/src/git-send-pull.bash > - Splitting up a large pull request into smaller ones can be > helpful to make sure things don't go in unnoticed. I try to > (briefly) look at each patch, but if you have 20 boring but > large patches, and a small but important patch that I may > need to comment on, that is a good reason to split. > >> Not too much to see here, Yang Yingliang has added some error handling >> to the setup of the driver that reports SiFive cache topology >> information. I've put it on -next given how far we are in the release >> cycle, feel free to put it on fixes if you disagree :) > > This is fine either way, as none of the fixes are likely to cause > any real issues. I usually like to err on the side of having too much > in the fixes branch instead of risking to miss something, but I'm > just as happy to follow your preference here. > >> RISC-V SoC drivers for v6.2 >> >> SiFive: >> - add probe error handling to the ccache driver > > Since this tag description becomes part of the git history, try to write > it like you would write a commit log in the future. Ideally that > avoids bulleted lists (I know they are easy) and instead uses full > sentences that explain things about the state of the patches. If there > are bugfixes, are users likely to need the fixes or were they found > through inspection? For new features, explain who would have the > corresponding hardware and what it does. Again, what you have here > is not wrong, but it can always get better. > > Arnd
On Tue, Nov 22, 2022 at 07:00:00AM -0800, Palmer Dabbelt wrote: > On Tue, 22 Nov 2022 06:38:40 PST (-0800), Arnd Bergmann wrote: > > On Mon, Nov 21, 2022, at 18:24, Conor Dooley wrote: > > > Hey Arnd, > > > > > > Same stuff applies here: lmk if there's something you'd rather see changed. > > > Perhaps you'd prefer to see PRs per vendor? Although I think that's less > > > likely to matter here than in the DT stuff. Again, I'll try to get the PR > > > out a bit earlier next time. > > > > Applied, this looks fine, just a few things to keep in mind: > > Thanks! Ditto :) > > - please add "[GIT PULL]" to the subject line of the email > > FWIW, here's the script I use to send pull requests: https://github.com/palmer-dabbelt/home/blob/master/.local/src/git-send-pull.bash I noticed that yesterday evening while double checking what I'd sent out but by then it was too late... I'll use the script I use for internal stuff going forward & hopefully avoid a repeat. > > - Splitting up a large pull request into smaller ones can be > > helpful to make sure things don't go in unnoticed. I try to > > (briefly) look at each patch, but if you have 20 boring but > > large patches, and a small but important patch that I may > > need to comment on, that is a good reason to split. Sure. I'll try to figure out what makes the most sense for a split versus having stuff in linux-next in a reasonable way. I suppose volume of changes will mainly dictate the approach. > > > Not too much to see here, Yang Yingliang has added some error handling > > > to the setup of the driver that reports SiFive cache topology > > > information. I've put it on -next given how far we are in the release > > > cycle, feel free to put it on fixes if you disagree :) > > > > This is fine either way, as none of the fixes are likely to cause > > any real issues. I usually like to err on the side of having too much > > in the fixes branch instead of risking to miss something, but I'm > > just as happy to follow your preference here. Cool. I'll bear that in mind for next time, thanks. > > > RISC-V SoC drivers for v6.2 > > > > > > SiFive: > > > - add probe error handling to the ccache driver > > > > Since this tag description becomes part of the git history, try to write > > it like you would write a commit log in the future. Ideally that > > avoids bulleted lists (I know they are easy) and instead uses full > > sentences that explain things about the state of the patches. If there > > are bugfixes, are users likely to need the fixes or were they found > > through inspection? For new features, explain who would have the > > corresponding hardware and what it does. Again, what you have here > > is not wrong, but it can always get better. Yeah, I can do that. You likely won't get a Christian Brauner novella out of me ever, but I'll do something more cover letter-y next time around. Thanks again, Conor.
On Tue, Nov 22, 2022, at 16:14, Conor Dooley wrote: > On Tue, Nov 22, 2022 at 07:00:00AM -0800, Palmer Dabbelt wrote: >> On Tue, 22 Nov 2022 06:38:40 PST (-0800), Arnd Bergmann wrote: >> > On Mon, Nov 21, 2022, at 18:24, Conor Dooley wrote: >> > - Splitting up a large pull request into smaller ones can be >> > helpful to make sure things don't go in unnoticed. I try to >> > (briefly) look at each patch, but if you have 20 boring but >> > large patches, and a small but important patch that I may >> > need to comment on, that is a good reason to split. > > Sure. I'll try to figure out what makes the most sense for a split > versus having stuff in linux-next in a reasonable way. I suppose volume > of changes will mainly dictate the approach. Note that you can split things two ways, either by time or by content: If you feel that the branch is getting a bit large, then use that as a hint that you should send out parts early, and follow up with another set of changes later, even if that touches the same files. Here I would expect later pull requests to be smaller than earlier ones for the same merge window. Splitting by content is different: If you feel that something in the branch is not like the other things, then you can split it in a way that would give each branch a sensible tag description rather than having to describe two different things in a coherent way. Often this ends up with one of them being much larger than the other, and that is ok. Arnd
Hey Arnd, Same stuff applies here: lmk if there's something you'd rather see changed. Perhaps you'd prefer to see PRs per vendor? Although I think that's less likely to matter here than in the DT stuff. Again, I'll try to get the PR out a bit earlier next time. Not too much to see here, Yang Yingliang has added some error handling to the setup of the driver that reports SiFive cache topology information. I've put it on -next given how far we are in the release cycle, feel free to put it on fixes if you disagree :) Thanks, Conor. The following changes since commit 9abf2313adc1ca1b6180c508c25f22f9395cc780: Linux 6.1-rc1 (2022-10-16 15:36:24 -0700) are available in the Git repository at: https://git.kernel.org/pub/scm/linux/kernel/git/conor/linux.git/ tags/riscv-soc-for-v6.2-mw0 for you to fetch changes up to 8fbf94fea0b4e187ca9100936c5429f96b8a4e44: soc: sifive: ccache: fix missing of_node_put() in sifive_ccache_init() (2022-11-09 22:01:31 +0000) ---------------------------------------------------------------- RISC-V SoC drivers for v6.2 SiFive: - add probe error handling to the ccache driver ---------------------------------------------------------------- Yang Yingliang (3): soc: sifive: ccache: fix missing iounmap() in error path in sifive_ccache_init() soc: sifive: ccache: fix missing free_irq() in error path in sifive_ccache_init() soc: sifive: ccache: fix missing of_node_put() in sifive_ccache_init() drivers/soc/sifive/sifive_ccache.c | 33 +++++++++++++++++++++++++-------- 1 file changed, 25 insertions(+), 8 deletions(-