| Message ID | 20240215171740.14550-3-fancer.lancer@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | MIPS: Fix missing proto and passing arg warnings | expand |
On Thu, Feb 15, 2024 at 08:17:27PM +0300, Serge Semin wrote: > Based on the design pattern utilized in the CM GCR and L2-sync base > address getters implementation the platform-specific code is capable to > re-define the getters and re-use the weakly defined initial versions. But > since the re-definition is supposed to be done in another source file the > interface methods have been globally defined which in its turn causes the > "no previous prototype" warning printed should the re-definition is > finally introduced. Since without the global declarations the pattern can > be considered as incomplete and causing the warning printed, fix it by > providing the respective methods prototype declarations in > "arch/mips/include/asm/mips-cm.h". > > Signed-off-by: Serge Semin <fancer.lancer@gmail.com> > > --- > > Note as I mentioned in the previous patch, since the weak implementation > of the getters isn't utilized other than as a default implementation of > the original methods, we can convert the denoted pattern to a simple > __weak attributed methods. Let me know if that would be more preferable. how about simply remove __mips_cm_l2sync_phys_base() and do everything via mips_cm_phys_base(). And at the moment without anyone overriding mips_cm_phys_base I tend to keep static without __weak. If someone needs, we can change it. Does this make sense ? Thomas.
On Tue, Feb 20, 2024 at 06:24:25PM +0100, Thomas Bogendoerfer wrote: > On Thu, Feb 15, 2024 at 08:17:27PM +0300, Serge Semin wrote: > > Based on the design pattern utilized in the CM GCR and L2-sync base > > address getters implementation the platform-specific code is capable to > > re-define the getters and re-use the weakly defined initial versions. But > > since the re-definition is supposed to be done in another source file the > > interface methods have been globally defined which in its turn causes the > > "no previous prototype" warning printed should the re-definition is > > finally introduced. Since without the global declarations the pattern can > > be considered as incomplete and causing the warning printed, fix it by > > providing the respective methods prototype declarations in > > "arch/mips/include/asm/mips-cm.h". > > > > Signed-off-by: Serge Semin <fancer.lancer@gmail.com> > > > > --- > > > > Note as I mentioned in the previous patch, since the weak implementation > > of the getters isn't utilized other than as a default implementation of > > the original methods, we can convert the denoted pattern to a simple > > __weak attributed methods. Let me know if that would be more preferable. > > how about simply remove __mips_cm_l2sync_phys_base() and do everything > via mips_cm_phys_base(). And at the moment without anyone overriding > mips_cm_phys_base I tend to keep static without __weak. If someone > needs, we can change it. Does this make sense ? To be honest my arch code (not submitted yet) do override the mips_cm_l2sync_phys_base() method. The memory just behind the CM2 region is hardwired for the EEPROM mapping. So the default __mips_cm_l2sync_phys_base() implementation selecting the L2-sync with (mips_cm_phys_base() + MIPS_CM_GCR_SIZE) isn't suitable for my platform. Note what you suggest will require the CM and CM L2-sync probe code logic redevelopment. The mips_cm_phys_base() method is currently dedicated for a single purpose: return the CM GCR physical base address. So is the mips_cm_l2sync_phys_base() method. Merging the later method into the former one will cause the mips_cm_phys_base() function looking less neat. We will also require to have the mips_cm_probe() method keeping the CM L2-sync physical base addresses around and then passing it to mips_cm_probe_l2sync() or having the later method embedded into mips_cm_probe(). All of that won't look as good as the current implementation. What about instead of that I'll just convert both mips_cm_phys_base() and mips_cm_l2sync_phys_base() to being defined with the underscored methods body and assign the __weak attribute to them? -Serge(y) > > Thomas. > > -- > Crap can work. Given enough thrust pigs will fly, but it's not necessarily a > good idea. [ RFC1925, 2.3 ]
On Wed, Feb 21, 2024 at 09:39:58PM +0300, Serge Semin wrote: > On Tue, Feb 20, 2024 at 06:24:25PM +0100, Thomas Bogendoerfer wrote: > > On Thu, Feb 15, 2024 at 08:17:27PM +0300, Serge Semin wrote: > > > Based on the design pattern utilized in the CM GCR and L2-sync base > > > address getters implementation the platform-specific code is capable to > > > re-define the getters and re-use the weakly defined initial versions. But > > > since the re-definition is supposed to be done in another source file the > > > interface methods have been globally defined which in its turn causes the > > > "no previous prototype" warning printed should the re-definition is > > > finally introduced. Since without the global declarations the pattern can > > > be considered as incomplete and causing the warning printed, fix it by > > > providing the respective methods prototype declarations in > > > "arch/mips/include/asm/mips-cm.h". > > > > > > Signed-off-by: Serge Semin <fancer.lancer@gmail.com> > > > > > > --- > > > > > > Note as I mentioned in the previous patch, since the weak implementation > > > of the getters isn't utilized other than as a default implementation of > > > the original methods, we can convert the denoted pattern to a simple > > > __weak attributed methods. Let me know if that would be more preferable. > > > > > how about simply remove __mips_cm_l2sync_phys_base() and do everything > > via mips_cm_phys_base(). And at the moment without anyone overriding > > mips_cm_phys_base I tend to keep static without __weak. If someone > > needs, we can change it. Does this make sense ? > > To be honest my arch code (not submitted yet) do override the > mips_cm_l2sync_phys_base() method. The memory just behind the CM2 that's fine, I just wanted to know a reason for having it provided as weak symbol. > What about instead of that I'll just convert both mips_cm_phys_base() > and mips_cm_l2sync_phys_base() to being defined with the underscored > methods body and assign the __weak attribute to them? works for me ;-) I'll pick patch 3/4 of this series, so no need to resend them. Thomas.
On Fri, Feb 23, 2024 at 10:06:33AM +0100, Thomas Bogendoerfer wrote: > On Wed, Feb 21, 2024 at 09:39:58PM +0300, Serge Semin wrote: > > On Tue, Feb 20, 2024 at 06:24:25PM +0100, Thomas Bogendoerfer wrote: > > > On Thu, Feb 15, 2024 at 08:17:27PM +0300, Serge Semin wrote: > > > > Based on the design pattern utilized in the CM GCR and L2-sync base > > > > address getters implementation the platform-specific code is capable to > > > > re-define the getters and re-use the weakly defined initial versions. But > > > > since the re-definition is supposed to be done in another source file the > > > > interface methods have been globally defined which in its turn causes the > > > > "no previous prototype" warning printed should the re-definition is > > > > finally introduced. Since without the global declarations the pattern can > > > > be considered as incomplete and causing the warning printed, fix it by > > > > providing the respective methods prototype declarations in > > > > "arch/mips/include/asm/mips-cm.h". > > > > > > > > Signed-off-by: Serge Semin <fancer.lancer@gmail.com> > > > > > > > > --- > > > > > > > > Note as I mentioned in the previous patch, since the weak implementation > > > > of the getters isn't utilized other than as a default implementation of > > > > the original methods, we can convert the denoted pattern to a simple > > > > __weak attributed methods. Let me know if that would be more preferable. > > > > > > > > how about simply remove __mips_cm_l2sync_phys_base() and do everything > > > via mips_cm_phys_base(). And at the moment without anyone overriding > > > mips_cm_phys_base I tend to keep static without __weak. If someone > > > needs, we can change it. Does this make sense ? > > > > To be honest my arch code (not submitted yet) do override the > > mips_cm_l2sync_phys_base() method. The memory just behind the CM2 > > that's fine, I just wanted to know a reason for having it provided as > weak symbol. > > > What about instead of that I'll just convert both mips_cm_phys_base() > > and mips_cm_l2sync_phys_base() to being defined with the underscored > > methods body and assign the __weak attribute to them? > > works for me ;-) I'll pick patch 3/4 of this series, so no need to > resend them. Ok. Thanks. I'll submit the respective patch(es) in a several days. -Serge(y) > > Thomas. > > -- > Crap can work. Given enough thrust pigs will fly, but it's not necessarily a > good idea. [ RFC1925, 2.3 ]
diff --git a/arch/mips/include/asm/mips-cm.h b/arch/mips/include/asm/mips-cm.h index 1f143dfad7a2..6dbe74dc323d 100644 --- a/arch/mips/include/asm/mips-cm.h +++ b/arch/mips/include/asm/mips-cm.h @@ -32,6 +32,7 @@ extern void __iomem *mips_cm_l2sync_base; * name mips_cm_phys_base (without underscores). */ extern phys_addr_t __mips_cm_phys_base(void); +extern phys_addr_t mips_cm_phys_base(void); /** * __mips_cm_l2sync_phys_base - retrieve the physical base address of the CM @@ -46,6 +47,7 @@ extern phys_addr_t __mips_cm_phys_base(void); * underscores). */ extern phys_addr_t __mips_cm_l2sync_phys_base(void); +extern phys_addr_t mips_cm_l2sync_phys_base(void); /* * mips_cm_is64 - determine CM register width
Based on the design pattern utilized in the CM GCR and L2-sync base address getters implementation the platform-specific code is capable to re-define the getters and re-use the weakly defined initial versions. But since the re-definition is supposed to be done in another source file the interface methods have been globally defined which in its turn causes the "no previous prototype" warning printed should the re-definition is finally introduced. Since without the global declarations the pattern can be considered as incomplete and causing the warning printed, fix it by providing the respective methods prototype declarations in "arch/mips/include/asm/mips-cm.h". Signed-off-by: Serge Semin <fancer.lancer@gmail.com> --- Note as I mentioned in the previous patch, since the weak implementation of the getters isn't utilized other than as a default implementation of the original methods, we can convert the denoted pattern to a simple __weak attributed methods. Let me know if that would be more preferable. --- arch/mips/include/asm/mips-cm.h | 2 ++ 1 file changed, 2 insertions(+)