Re: [PATCH 4/4] powerpc/mpic: remove unused functions
> @@ -1676,31 +1666,6 @@ void __init mpic_init(struct mpic *mpic)
> mpic_err_int_init(mpic, MPIC_FSL_ERR_INT);
> }
>
> -void __init mpic_set_clk_ratio(struct mpic *mpic, u32 clock_ratio)
> -{
> - u32 v;
> -
> - v = mpic_read(mpic->gregs, MPIC_GREG_GLOBAL_CONF_1);
> - v &= ~MPIC_GREG_GLOBAL_CONF_1_CLK_RATIO_MASK;
> - v |= MPIC_GREG_GLOBAL_CONF_1_CLK_RATIO(clock_ratio);
> - mpic_write(mpic->gregs, MPIC_GREG_GLOBAL_CONF_1, v);
> -}
> -
> -void __init mpic_set_serial_int(struct mpic *mpic, int enable)
> -{
> - unsigned long flags;
> - u32 v;
> -
> - raw_spin_lock_irqsave(_lock, flags);
> - v = mpic_read(mpic->gregs, MPIC_GREG_GLOBAL_CONF_1);
> - if (enable)
> - v |= MPIC_GREG_GLOBAL_CONF_1_SIE;
> - else
> - v &= ~MPIC_GREG_GLOBAL_CONF_1_SIE;
> - mpic_write(mpic->gregs, MPIC_GREG_GLOBAL_CONF_1, v);
> - raw_spin_unlock_irqrestore(_lock, flags);
> -}
> -
> void mpic_irq_set_priority(unsigned int irq, unsigned int pri)
> {
> struct mpic *mpic = mpic_find(irq);
Thinking about it some more, I wonder whether it makes sense to propagate these
values through device tree (and refuse to apply them if they are 0), just like
what timer_group_get_freq() and timer_group_get_irq() in
arch/powerpc/sysdev/mpic_timer.c do.
Does it have any real use?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/4] powerpc/mpic: remove unused functions
@@ -1676,31 +1666,6 @@ void __init mpic_init(struct mpic *mpic)
mpic_err_int_init(mpic, MPIC_FSL_ERR_INT);
}
-void __init mpic_set_clk_ratio(struct mpic *mpic, u32 clock_ratio)
-{
- u32 v;
-
- v = mpic_read(mpic-gregs, MPIC_GREG_GLOBAL_CONF_1);
- v = ~MPIC_GREG_GLOBAL_CONF_1_CLK_RATIO_MASK;
- v |= MPIC_GREG_GLOBAL_CONF_1_CLK_RATIO(clock_ratio);
- mpic_write(mpic-gregs, MPIC_GREG_GLOBAL_CONF_1, v);
-}
-
-void __init mpic_set_serial_int(struct mpic *mpic, int enable)
-{
- unsigned long flags;
- u32 v;
-
- raw_spin_lock_irqsave(mpic_lock, flags);
- v = mpic_read(mpic-gregs, MPIC_GREG_GLOBAL_CONF_1);
- if (enable)
- v |= MPIC_GREG_GLOBAL_CONF_1_SIE;
- else
- v = ~MPIC_GREG_GLOBAL_CONF_1_SIE;
- mpic_write(mpic-gregs, MPIC_GREG_GLOBAL_CONF_1, v);
- raw_spin_unlock_irqrestore(mpic_lock, flags);
-}
-
void mpic_irq_set_priority(unsigned int irq, unsigned int pri)
{
struct mpic *mpic = mpic_find(irq);
Thinking about it some more, I wonder whether it makes sense to propagate these
values through device tree (and refuse to apply them if they are 0), just like
what timer_group_get_freq() and timer_group_get_irq() in
arch/powerpc/sysdev/mpic_timer.c do.
Does it have any real use?
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/4] powerpc/mpic: remove unused functions
On Wed, 2015-02-25 at 20:39 -0600, Jia Hongtao-B38951 wrote: > Hi Scott, > > I'm really sorry for leave this patch like a zombie. > Now I have plan to revisit this patch. > > From the previous comments the compile error was fixed. > But beyond that I have had no plan to update it. > > Could you please comment on why it's still on hold? > Kumar had some comments. -Scott -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 4/4] powerpc/mpic: remove unused functions
Hi Scott, I'm really sorry for leave this patch like a zombie. Now I have plan to revisit this patch. From the previous comments the compile error was fixed. But beyond that I have had no plan to update it. Could you please comment on why it's still on hold? Thanks. > -Original Message- > From: Wood Scott-B07421 > Sent: Tuesday, February 24, 2015 5:32 AM > To: Arseny Solokha > Cc: Michael Ellerman; Benjamin Herrenschmidt; Paul Mackerras; linuxppc- > d...@lists.ozlabs.org; linux-kernel@vger.kernel.org; Jia Hongtao-B38951 > Subject: Re: [PATCH 4/4] powerpc/mpic: remove unused functions > > On Thu, 2015-02-19 at 19:26 +0700, Arseny Solokha wrote: > > + fsl_mpic_primary_get_version() is just a safe wrapper around > > fsl_mpic_get_version() for SMP configurations. While the latter is > > called explicitly for handling PIC initialization and setting up error > > interrupt vector depending on PIC hardware version, the former isn't > > used for anything. > > It was meant to be used by http://patchwork.ozlabs.org/patch/233211/ > which never got respun. Hongtao, do you plan to revisit that patch? > > -Scott >
RE: [PATCH 4/4] powerpc/mpic: remove unused functions
Hi Scott, I'm really sorry for leave this patch like a zombie. Now I have plan to revisit this patch. From the previous comments the compile error was fixed. But beyond that I have had no plan to update it. Could you please comment on why it's still on hold? Thanks. -Original Message- From: Wood Scott-B07421 Sent: Tuesday, February 24, 2015 5:32 AM To: Arseny Solokha Cc: Michael Ellerman; Benjamin Herrenschmidt; Paul Mackerras; linuxppc- d...@lists.ozlabs.org; linux-kernel@vger.kernel.org; Jia Hongtao-B38951 Subject: Re: [PATCH 4/4] powerpc/mpic: remove unused functions On Thu, 2015-02-19 at 19:26 +0700, Arseny Solokha wrote: + fsl_mpic_primary_get_version() is just a safe wrapper around fsl_mpic_get_version() for SMP configurations. While the latter is called explicitly for handling PIC initialization and setting up error interrupt vector depending on PIC hardware version, the former isn't used for anything. It was meant to be used by http://patchwork.ozlabs.org/patch/233211/ which never got respun. Hongtao, do you plan to revisit that patch? -Scott
Re: [PATCH 4/4] powerpc/mpic: remove unused functions
On Wed, 2015-02-25 at 20:39 -0600, Jia Hongtao-B38951 wrote: Hi Scott, I'm really sorry for leave this patch like a zombie. Now I have plan to revisit this patch. From the previous comments the compile error was fixed. But beyond that I have had no plan to update it. Could you please comment on why it's still on hold? Kumar had some comments. -Scott -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/4] powerpc/mpic: remove unused functions
On Fri, 2015-02-20 at 11:40 +0700, Arseny Solokha wrote: > > If I just get a patch saying "removed unused foo()", I have to go and dig > > and > > find out: > > - was it recently added and will be used soon? > > - is it ancient and never used, if so can we work out why, ie. feature X > > never landed so this code is no longer needed. > > - is it old code that *was* used but isn't now because commit ... removed > > the > > last user. > > - is it code that *should* be used, but isn't for some odd reason? > > > > > > So if you can provide that sort of detail for me, that really adds value to > > the > > patch. Otherwise the patch is basically just a TODO for me, to go and work > > out > > why the code is unused. > > Got it. Will resend the whole series one of these days. Thanks. cheers -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/4] powerpc/mpic: remove unused functions
On Thu, 2015-02-19 at 19:26 +0700, Arseny Solokha wrote: > + fsl_mpic_primary_get_version() is just a safe wrapper around > fsl_mpic_get_version() for SMP configurations. While the latter is > called explicitly for handling PIC initialization and setting up error > interrupt vector depending on PIC hardware version, the former isn't > used for anything. It was meant to be used by http://patchwork.ozlabs.org/patch/233211/ which never got respun. Hongtao, do you plan to revisit that patch? -Scott -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/4] powerpc/mpic: remove unused functions
On Thu, 2015-02-19 at 19:26 +0700, Arseny Solokha wrote: + fsl_mpic_primary_get_version() is just a safe wrapper around fsl_mpic_get_version() for SMP configurations. While the latter is called explicitly for handling PIC initialization and setting up error interrupt vector depending on PIC hardware version, the former isn't used for anything. It was meant to be used by http://patchwork.ozlabs.org/patch/233211/ which never got respun. Hongtao, do you plan to revisit that patch? -Scott -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/4] powerpc/mpic: remove unused functions
On Fri, 2015-02-20 at 11:40 +0700, Arseny Solokha wrote: If I just get a patch saying removed unused foo(), I have to go and dig and find out: - was it recently added and will be used soon? - is it ancient and never used, if so can we work out why, ie. feature X never landed so this code is no longer needed. - is it old code that *was* used but isn't now because commit ... removed the last user. - is it code that *should* be used, but isn't for some odd reason? So if you can provide that sort of detail for me, that really adds value to the patch. Otherwise the patch is basically just a TODO for me, to go and work out why the code is unused. Got it. Will resend the whole series one of these days. Thanks. cheers -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/4] powerpc/mpic: remove unused functions
> If I just get a patch saying "removed unused foo()", I have to go and dig and > find out: > - was it recently added and will be used soon? > - is it ancient and never used, if so can we work out why, ie. feature X > never landed so this code is no longer needed. > - is it old code that *was* used but isn't now because commit ... removed > the > last user. > - is it code that *should* be used, but isn't for some odd reason? > > > So if you can provide that sort of detail for me, that really adds value to > the > patch. Otherwise the patch is basically just a TODO for me, to go and work out > why the code is unused. Got it. Will resend the whole series one of these days. Arsény > cheers -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/4] powerpc/mpic: remove unused functions
On Thu, 2015-02-19 at 19:26 +0700, Arseny Solokha wrote: > > On Mon, 2015-02-16 at 17:56 +0700, Arseny Solokha wrote: > >> Drop unused fsl_mpic_primary_get_version(), mpic_set_clk_ratio(), > >> mpic_set_serial_int(). > > > > I'm always happy to remove unused code, but the interesting question is why > > are > > they unused? Please tell me in the changelog. > > To being able to give a definitive answer, it's necessary to understand > the intentions of original developers of these pieces. I just can tell > these functions have no users and trivial grepping easily proves it; > I've got the impression they are here only for the sake of > implementation completeness. Yeah OK. I didn't expect you to read the minds of the developers who wrote the code :) > Two machines at hands, e300 and e500 based, boot and run without > regressions on my workload with this series applied. The removed code > seems also been rarely touched, so it seems the series is safe at least > in general. But I can't obviously express any strong point in support of > the series, so it's completely OK to leave things as is. OK that's a good data point. > + fsl_mpic_primary_get_version() is just a safe wrapper around > fsl_mpic_get_version() for SMP configurations. While the latter is > called explicitly for handling PIC initialization and setting up error > interrupt vector depending on PIC hardware version, the former isn't > used for anything. > > + As for mpic_set_clk_ratio() and mpic_set_serial_int(), they both > are almost nine years old[1] but still have no chance to be called even > from out-of-tree modules because they both are __init and of course > aren't exported. Non-demanded functionality? > > Of course I'll include the last two paragraphs into the V2 patch > description if the explanation is convincing enough and you ACK it. If > the patch is safe it's also necessary to extend it a bit, making its > second part actually a complete revert of [1]. > > [1] https://lists.ozlabs.org/pipermail/linuxppc-dev/2006-June/023867.html That is more like what I was looking for. If I just get a patch saying "removed unused foo()", I have to go and dig and find out: - was it recently added and will be used soon? - is it ancient and never used, if so can we work out why, ie. feature X never landed so this code is no longer needed. - is it old code that *was* used but isn't now because commit ... removed the last user. - is it code that *should* be used, but isn't for some odd reason? So if you can provide that sort of detail for me, that really adds value to the patch. Otherwise the patch is basically just a TODO for me, to go and work out why the code is unused. cheers -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/4] powerpc/mpic: remove unused functions
> On Mon, 2015-02-16 at 17:56 +0700, Arseny Solokha wrote: >> Drop unused fsl_mpic_primary_get_version(), mpic_set_clk_ratio(), >> mpic_set_serial_int(). > > I'm always happy to remove unused code, but the interesting question is why > are > they unused? Please tell me in the changelog. To being able to give a definitive answer, it's necessary to understand the intentions of original developers of these pieces. I just can tell these functions have no users and trivial grepping easily proves it; I've got the impression they are here only for the sake of implementation completeness. Two machines at hands, e300 and e500 based, boot and run without regressions on my workload with this series applied. The removed code seems also been rarely touched, so it seems the series is safe at least in general. But I can't obviously express any strong point in support of the series, so it's completely OK to leave things as is. + fsl_mpic_primary_get_version() is just a safe wrapper around fsl_mpic_get_version() for SMP configurations. While the latter is called explicitly for handling PIC initialization and setting up error interrupt vector depending on PIC hardware version, the former isn't used for anything. + As for mpic_set_clk_ratio() and mpic_set_serial_int(), they both are almost nine years old[1] but still have no chance to be called even from out-of-tree modules because they both are __init and of course aren't exported. Non-demanded functionality? Of course I'll include the last two paragraphs into the V2 patch description if the explanation is convincing enough and you ACK it. If the patch is safe it's also necessary to extend it a bit, making its second part actually a complete revert of [1]. [1] https://lists.ozlabs.org/pipermail/linuxppc-dev/2006-June/023867.html Arsény > cheers -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/4] powerpc/mpic: remove unused functions
On Mon, 2015-02-16 at 17:56 +0700, Arseny Solokha wrote: Drop unused fsl_mpic_primary_get_version(), mpic_set_clk_ratio(), mpic_set_serial_int(). I'm always happy to remove unused code, but the interesting question is why are they unused? Please tell me in the changelog. To being able to give a definitive answer, it's necessary to understand the intentions of original developers of these pieces. I just can tell these functions have no users and trivial grepping easily proves it; I've got the impression they are here only for the sake of implementation completeness. Two machines at hands, e300 and e500 based, boot and run without regressions on my workload with this series applied. The removed code seems also been rarely touched, so it seems the series is safe at least in general. But I can't obviously express any strong point in support of the series, so it's completely OK to leave things as is. + fsl_mpic_primary_get_version() is just a safe wrapper around fsl_mpic_get_version() for SMP configurations. While the latter is called explicitly for handling PIC initialization and setting up error interrupt vector depending on PIC hardware version, the former isn't used for anything. + As for mpic_set_clk_ratio() and mpic_set_serial_int(), they both are almost nine years old[1] but still have no chance to be called even from out-of-tree modules because they both are __init and of course aren't exported. Non-demanded functionality? Of course I'll include the last two paragraphs into the V2 patch description if the explanation is convincing enough and you ACK it. If the patch is safe it's also necessary to extend it a bit, making its second part actually a complete revert of [1]. [1] https://lists.ozlabs.org/pipermail/linuxppc-dev/2006-June/023867.html Arsény cheers -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/4] powerpc/mpic: remove unused functions
If I just get a patch saying removed unused foo(), I have to go and dig and find out: - was it recently added and will be used soon? - is it ancient and never used, if so can we work out why, ie. feature X never landed so this code is no longer needed. - is it old code that *was* used but isn't now because commit ... removed the last user. - is it code that *should* be used, but isn't for some odd reason? So if you can provide that sort of detail for me, that really adds value to the patch. Otherwise the patch is basically just a TODO for me, to go and work out why the code is unused. Got it. Will resend the whole series one of these days. Arsény cheers -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/4] powerpc/mpic: remove unused functions
On Thu, 2015-02-19 at 19:26 +0700, Arseny Solokha wrote: On Mon, 2015-02-16 at 17:56 +0700, Arseny Solokha wrote: Drop unused fsl_mpic_primary_get_version(), mpic_set_clk_ratio(), mpic_set_serial_int(). I'm always happy to remove unused code, but the interesting question is why are they unused? Please tell me in the changelog. To being able to give a definitive answer, it's necessary to understand the intentions of original developers of these pieces. I just can tell these functions have no users and trivial grepping easily proves it; I've got the impression they are here only for the sake of implementation completeness. Yeah OK. I didn't expect you to read the minds of the developers who wrote the code :) Two machines at hands, e300 and e500 based, boot and run without regressions on my workload with this series applied. The removed code seems also been rarely touched, so it seems the series is safe at least in general. But I can't obviously express any strong point in support of the series, so it's completely OK to leave things as is. OK that's a good data point. + fsl_mpic_primary_get_version() is just a safe wrapper around fsl_mpic_get_version() for SMP configurations. While the latter is called explicitly for handling PIC initialization and setting up error interrupt vector depending on PIC hardware version, the former isn't used for anything. + As for mpic_set_clk_ratio() and mpic_set_serial_int(), they both are almost nine years old[1] but still have no chance to be called even from out-of-tree modules because they both are __init and of course aren't exported. Non-demanded functionality? Of course I'll include the last two paragraphs into the V2 patch description if the explanation is convincing enough and you ACK it. If the patch is safe it's also necessary to extend it a bit, making its second part actually a complete revert of [1]. [1] https://lists.ozlabs.org/pipermail/linuxppc-dev/2006-June/023867.html That is more like what I was looking for. If I just get a patch saying removed unused foo(), I have to go and dig and find out: - was it recently added and will be used soon? - is it ancient and never used, if so can we work out why, ie. feature X never landed so this code is no longer needed. - is it old code that *was* used but isn't now because commit ... removed the last user. - is it code that *should* be used, but isn't for some odd reason? So if you can provide that sort of detail for me, that really adds value to the patch. Otherwise the patch is basically just a TODO for me, to go and work out why the code is unused. cheers -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/4] powerpc/mpic: remove unused functions
On Mon, 2015-02-16 at 17:56 +0700, Arseny Solokha wrote: > Drop unused fsl_mpic_primary_get_version(), mpic_set_clk_ratio(), > mpic_set_serial_int(). I'm always happy to remove unused code, but the interesting question is why are they unused? Please tell me in the changelog. cheers -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/4] powerpc/mpic: remove unused functions
On Mon, 2015-02-16 at 17:56 +0700, Arseny Solokha wrote: Drop unused fsl_mpic_primary_get_version(), mpic_set_clk_ratio(), mpic_set_serial_int(). I'm always happy to remove unused code, but the interesting question is why are they unused? Please tell me in the changelog. cheers -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
