| Message ID | 126bd153a03f39e42645573eecf44ffab5354fc7.1718791090.git.u.kleine-koenig@baylibre.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | mfd: stm32-timers: Make register definition more flexible | expand |
On Wed, 19 Jun 2024, Uwe Kleine-König wrote: > These two defines have the same purpose and this change doesn't > introduce any differences in drivers/counter/stm32-timer-cnt.o. > > The only difference between the two is that > > TIM_DIER_CC_IE(1) == TIM_DIER_CC2IE > > while > > TIM_DIER_CCxIE(1) == TIM_DIER_CC1IE > > . That makes it necessary to have an explicit "+ 1" in the user code, > but IMHO this is a good thing as this is the code locatation that > "knows" that for software channel 1 you have to use TIM_DIER_CC2IE > (because software guys start counting at 0, while the relevant hardware > designer started at 1). > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com> > --- > drivers/counter/stm32-timer-cnt.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) Did you drop William's Ack on purpose?
On Thu, Jun 20, 2024 at 09:44:51AM +0100, Lee Jones wrote: > On Wed, 19 Jun 2024, Uwe Kleine-König wrote: > > > These two defines have the same purpose and this change doesn't > > introduce any differences in drivers/counter/stm32-timer-cnt.o. > > > > The only difference between the two is that > > > > TIM_DIER_CC_IE(1) == TIM_DIER_CC2IE > > > > while > > > > TIM_DIER_CCxIE(1) == TIM_DIER_CC1IE > > > > . That makes it necessary to have an explicit "+ 1" in the user code, > > but IMHO this is a good thing as this is the code locatation that > > "knows" that for software channel 1 you have to use TIM_DIER_CC2IE > > (because software guys start counting at 0, while the relevant hardware > > designer started at 1). > > > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com> > > --- > > drivers/counter/stm32-timer-cnt.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > Did you drop William's Ack on purpose? > > -- > Lee Jones [李琼斯] No problem, here it is again for the sake of the LKML scraper tools: Acked-by: William Breathitt Gray <wbg@kernel.org> Lee, do you prefer taking this patchset through your tree? William Breathitt Gray
On Thu, 20 Jun 2024, William Breathitt Gray wrote: > On Thu, Jun 20, 2024 at 09:44:51AM +0100, Lee Jones wrote: > > On Wed, 19 Jun 2024, Uwe Kleine-König wrote: > > > > > These two defines have the same purpose and this change doesn't > > > introduce any differences in drivers/counter/stm32-timer-cnt.o. > > > > > > The only difference between the two is that > > > > > > TIM_DIER_CC_IE(1) == TIM_DIER_CC2IE > > > > > > while > > > > > > TIM_DIER_CCxIE(1) == TIM_DIER_CC1IE > > > > > > . That makes it necessary to have an explicit "+ 1" in the user code, > > > but IMHO this is a good thing as this is the code locatation that > > > "knows" that for software channel 1 you have to use TIM_DIER_CC2IE > > > (because software guys start counting at 0, while the relevant hardware > > > designer started at 1). > > > > > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com> > > > --- > > > drivers/counter/stm32-timer-cnt.c | 4 ++-- > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > Did you drop William's Ack on purpose? > > > > -- > > Lee Jones [李琼斯] > > No problem, here it is again for the sake of the LKML scraper tools: > > Acked-by: William Breathitt Gray <wbg@kernel.org> > > Lee, do you prefer taking this patchset through your tree? I think that would make things easier. A pull-request for you and the PWM folk would follow.
Hello Lee, On Thu, Jun 20, 2024 at 09:44:51AM +0100, Lee Jones wrote: > On Wed, 19 Jun 2024, Uwe Kleine-König wrote: > > > These two defines have the same purpose and this change doesn't > > introduce any differences in drivers/counter/stm32-timer-cnt.o. > > > > The only difference between the two is that > > > > TIM_DIER_CC_IE(1) == TIM_DIER_CC2IE > > > > while > > > > TIM_DIER_CCxIE(1) == TIM_DIER_CC1IE > > > > . That makes it necessary to have an explicit "+ 1" in the user code, > > but IMHO this is a good thing as this is the code locatation that > > "knows" that for software channel 1 you have to use TIM_DIER_CC2IE > > (because software guys start counting at 0, while the relevant hardware > > designer started at 1). > > > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com> > > --- > > drivers/counter/stm32-timer-cnt.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > Did you drop William's Ack on purpose? Yes, because a) I was unsure what he didn't like about the subject, and (more importantly) b) I split the patch in question. I should have written that in the cover letter, sorry. (Note I only announced to have fixed the subject prefix of the pwm patch. I assume you won't include that in your pull request, but if you do, please do s/-/: / on it. That's another thing I failed with for this series.) Best regards Uwe
On Thu, 20 Jun 2024, Uwe Kleine-König wrote: > Hello Lee, > > On Thu, Jun 20, 2024 at 09:44:51AM +0100, Lee Jones wrote: > > On Wed, 19 Jun 2024, Uwe Kleine-König wrote: > > > > > These two defines have the same purpose and this change doesn't > > > introduce any differences in drivers/counter/stm32-timer-cnt.o. > > > > > > The only difference between the two is that > > > > > > TIM_DIER_CC_IE(1) == TIM_DIER_CC2IE > > > > > > while > > > > > > TIM_DIER_CCxIE(1) == TIM_DIER_CC1IE > > > > > > . That makes it necessary to have an explicit "+ 1" in the user code, > > > but IMHO this is a good thing as this is the code locatation that > > > "knows" that for software channel 1 you have to use TIM_DIER_CC2IE > > > (because software guys start counting at 0, while the relevant hardware > > > designer started at 1). > > > > > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com> > > > --- > > > drivers/counter/stm32-timer-cnt.c | 4 ++-- > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > Did you drop William's Ack on purpose? > > Yes, because a) I was unsure what he didn't like about the subject, and > (more importantly) b) I split the patch in question. I should have > written that in the cover letter, sorry. > > (Note I only announced to have fixed the subject prefix of the pwm > patch. I assume you won't include that in your pull request, but if you > do, please do s/-/: / on it. That's another thing I failed with for this > series.) Which patches need to be in the pull-request and which can be hoovered up by their associated subsystems?
Hello Lee, On Thu, Jun 20, 2024 at 06:38:38PM +0100, Lee Jones wrote: > Which patches need to be in the pull-request and which can be hoovered > up by their associated subsystems? The dependencies are as follows: #1 <- #2 <- #3 <- #4 ^ #5 So the practical options are (in the order of my not very strong preference): - Immutable branch with #1 - #4 (assuming William is ok to let you merge #3), and then I can add #5 (and further work on the pwm-stm32 driver that I'm currently working on). Maybe William even doesn't need to pull; I didn't check. - Immutable branch with only #1 and #2. Then William can pull and add #3 and I can pull and add #5. #4 can be applied later then. (I can remind about #4 in the next cycle.) - Immutable branch with #1 - #5 (Reminder: In that case please fixup the pwm patch's short log with s/-/: /) I would add this for sure to the pwm tree. I didn't even try to check if it's needed for mfd and/or counter. So if you don't need it, I volunteer to create that branch, but if you want to do it, that's just fine, too. Best regards Uwe
Hello, On Thu, Jun 20, 2024 at 10:56:15PM +0200, Uwe Kleine-König wrote: > On Thu, Jun 20, 2024 at 06:38:38PM +0100, Lee Jones wrote: > > Which patches need to be in the pull-request and which can be hoovered > > up by their associated subsystems? > > The dependencies are as follows: > > #1 <- #2 <- #3 <- #4 > ^ > #5 > > So the practical options are (in the order of my not very strong preference): > > - Immutable branch with #1 - #4 (assuming William is ok to let you > merge #3), and then I can add #5 (and further work on the pwm-stm32 > driver that I'm currently working on). Maybe William even doesn't > need to pull; I didn't check. > > - Immutable branch with only #1 and #2. Then William can pull and add > #3 and I can pull and add #5. #4 can be applied later then. > (I can remind about #4 in the next cycle.) > > - Immutable branch with #1 - #5 > (Reminder: In that case please fixup the pwm patch's short log with > s/-/: /) > I would add this for sure to the pwm tree. I didn't even try to check > if it's needed for mfd and/or counter. So if you don't need it, I > volunteer to create that branch, but if you want to do it, that's > just fine, too. I wonder what's the state here. Maybe Lee waiting for William to send an Ack that Lee can do the first option? Best regards Uwe
Hello, On Wed, Jun 26, 2024 at 09:43:29PM +0200, Uwe Kleine-König wrote: > I wonder what's the state here. Maybe Lee waiting for William to send an > Ack that Lee can do the first option? Huh, sorry for the noise, just noticed Lee's mail after sending mine ... Best regards Uwe
diff --git a/drivers/counter/stm32-timer-cnt.c b/drivers/counter/stm32-timer-cnt.c index 0664ef969f79..186e73d6ccb4 100644 --- a/drivers/counter/stm32-timer-cnt.c +++ b/drivers/counter/stm32-timer-cnt.c @@ -465,7 +465,7 @@ static int stm32_count_events_configure(struct counter_device *counter) ret = stm32_count_capture_configure(counter, event_node->channel, true); if (ret) return ret; - dier |= TIM_DIER_CC_IE(event_node->channel); + dier |= TIM_DIER_CCxIE(event_node->channel + 1); break; default: /* should never reach this path */ @@ -478,7 +478,7 @@ static int stm32_count_events_configure(struct counter_device *counter) /* check for disabled capture events */ for (i = 0 ; i < priv->nchannels; i++) { - if (!(dier & TIM_DIER_CC_IE(i))) { + if (!(dier & TIM_DIER_CCxIE(i + 1))) { ret = stm32_count_capture_configure(counter, i, false); if (ret) return ret;
These two defines have the same purpose and this change doesn't introduce any differences in drivers/counter/stm32-timer-cnt.o. The only difference between the two is that TIM_DIER_CC_IE(1) == TIM_DIER_CC2IE while TIM_DIER_CCxIE(1) == TIM_DIER_CC1IE . That makes it necessary to have an explicit "+ 1" in the user code, but IMHO this is a good thing as this is the code locatation that "knows" that for software channel 1 you have to use TIM_DIER_CC2IE (because software guys start counting at 0, while the relevant hardware designer started at 1). Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com> --- drivers/counter/stm32-timer-cnt.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)