| Message ID | 20231211-b4-dwc3-qcom-v1-1-46275113b4f2@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | Qualcomm quirky SMMU support | expand |
On 2023-12-11 19:41, Caleb Connolly wrote: > The dev_pci_iommu_enable() function is only available when CONFIG_PCI > is > enabled, replace the runtime check with a preprocessor one to fix > compilation with pci disabled. > > Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org> > --- > drivers/iommu/iommu-uclass.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/iommu/iommu-uclass.c > b/drivers/iommu/iommu-uclass.c > index 72f123df55a5..98731d5e2c44 100644 > --- a/drivers/iommu/iommu-uclass.c > +++ b/drivers/iommu/iommu-uclass.c > @@ -100,9 +100,10 @@ int dev_iommu_enable(struct udevice *dev) > dev->iommu = dev_iommu; > } > > - if (CONFIG_IS_ENABLED(PCI) && count < 0 && > - device_is_on_pci_bus(dev)) > +#if CONFIG_IS_ENABLED(PCI) > + if (count < 0 && device_is_on_pci_bus(dev)) > return dev_pci_iommu_enable(dev); > +#endif > > return 0; > } Perhaps there's no need to introduce an ifdef here.
On Mon, Dec 11, 2023 at 08:08:32PM +0100, Dragan Simic wrote: > On 2023-12-11 19:41, Caleb Connolly wrote: > > The dev_pci_iommu_enable() function is only available when CONFIG_PCI is > > enabled, replace the runtime check with a preprocessor one to fix > > compilation with pci disabled. > > > > Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org> > > --- > > drivers/iommu/iommu-uclass.c | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/iommu/iommu-uclass.c b/drivers/iommu/iommu-uclass.c > > index 72f123df55a5..98731d5e2c44 100644 > > --- a/drivers/iommu/iommu-uclass.c > > +++ b/drivers/iommu/iommu-uclass.c > > @@ -100,9 +100,10 @@ int dev_iommu_enable(struct udevice *dev) > > dev->iommu = dev_iommu; > > } > > > > - if (CONFIG_IS_ENABLED(PCI) && count < 0 && > > - device_is_on_pci_bus(dev)) > > +#if CONFIG_IS_ENABLED(PCI) > > + if (count < 0 && device_is_on_pci_bus(dev)) > > return dev_pci_iommu_enable(dev); > > +#endif > > > > return 0; > > } > > Perhaps there's no need to introduce an ifdef here. Yes, how exactly are you getting a build failure? dev_pci_iommu_enable should be available and return false with CONFIG_PCI=n.
On 11/12/2023 19:17, Tom Rini wrote: > On Mon, Dec 11, 2023 at 08:08:32PM +0100, Dragan Simic wrote: >> On 2023-12-11 19:41, Caleb Connolly wrote: >>> The dev_pci_iommu_enable() function is only available when CONFIG_PCI is >>> enabled, replace the runtime check with a preprocessor one to fix >>> compilation with pci disabled. >>> >>> Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org> >>> --- >>> drivers/iommu/iommu-uclass.c | 5 +++-- >>> 1 file changed, 3 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/iommu/iommu-uclass.c b/drivers/iommu/iommu-uclass.c >>> index 72f123df55a5..98731d5e2c44 100644 >>> --- a/drivers/iommu/iommu-uclass.c >>> +++ b/drivers/iommu/iommu-uclass.c >>> @@ -100,9 +100,10 @@ int dev_iommu_enable(struct udevice *dev) >>> dev->iommu = dev_iommu; >>> } >>> >>> - if (CONFIG_IS_ENABLED(PCI) && count < 0 && >>> - device_is_on_pci_bus(dev)) >>> +#if CONFIG_IS_ENABLED(PCI) >>> + if (count < 0 && device_is_on_pci_bus(dev)) >>> return dev_pci_iommu_enable(dev); >>> +#endif >>> >>> return 0; >>> } >> >> Perhaps there's no need to introduce an ifdef here. > > Yes, how exactly are you getting a build failure? dev_pci_iommu_enable > should be available and return false with CONFIG_PCI=n. Hi, Without this patch I get ../drivers/iommu/iommu-uclass.c: In function 'dev_iommu_enable': ../drivers/iommu/iommu-uclass.c:116:24: warning: implicit declaration of function 'dev_pci_iommu_enable'; did you mean 'dev_iommu_enable'? [-Wimplicit-function-declaration] 116 | return dev_pci_iommu_enable(dev); | ^~~~~~~~~~~~~~~~~~~~ | dev_iommu_enable Grepping shows there is only one definition, which is the static function definition in iommu-uclass which is #ifdef'd out when CONFIG_PCI is disabled. ; rg "dev_pci_iommu_enable" drivers/iommu/iommu-uclass.c 18:static int dev_pci_iommu_enable(struct udevice *dev) 116: return dev_pci_iommu_enable(dev); > Am I missing a patch or something? The function is only defined in one place in the whole of U-Boot.
On Mon, Dec 11, 2023 at 07:47:04PM +0000, Caleb Connolly wrote: > > > On 11/12/2023 19:17, Tom Rini wrote: > > On Mon, Dec 11, 2023 at 08:08:32PM +0100, Dragan Simic wrote: > >> On 2023-12-11 19:41, Caleb Connolly wrote: > >>> The dev_pci_iommu_enable() function is only available when CONFIG_PCI is > >>> enabled, replace the runtime check with a preprocessor one to fix > >>> compilation with pci disabled. > >>> > >>> Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org> > >>> --- > >>> drivers/iommu/iommu-uclass.c | 5 +++-- > >>> 1 file changed, 3 insertions(+), 2 deletions(-) > >>> > >>> diff --git a/drivers/iommu/iommu-uclass.c b/drivers/iommu/iommu-uclass.c > >>> index 72f123df55a5..98731d5e2c44 100644 > >>> --- a/drivers/iommu/iommu-uclass.c > >>> +++ b/drivers/iommu/iommu-uclass.c > >>> @@ -100,9 +100,10 @@ int dev_iommu_enable(struct udevice *dev) > >>> dev->iommu = dev_iommu; > >>> } > >>> > >>> - if (CONFIG_IS_ENABLED(PCI) && count < 0 && > >>> - device_is_on_pci_bus(dev)) > >>> +#if CONFIG_IS_ENABLED(PCI) > >>> + if (count < 0 && device_is_on_pci_bus(dev)) > >>> return dev_pci_iommu_enable(dev); > >>> +#endif > >>> > >>> return 0; > >>> } > >> > >> Perhaps there's no need to introduce an ifdef here. > > > > Yes, how exactly are you getting a build failure? dev_pci_iommu_enable > > should be available and return false with CONFIG_PCI=n. > > Hi, > > Without this patch I get > > ../drivers/iommu/iommu-uclass.c: In function 'dev_iommu_enable': > ../drivers/iommu/iommu-uclass.c:116:24: warning: implicit declaration of > function 'dev_pci_iommu_enable'; did you mean 'dev_iommu_enable'? > [-Wimplicit-function-declaration] > 116 | return dev_pci_iommu_enable(dev); > | ^~~~~~~~~~~~~~~~~~~~ > | dev_iommu_enable > > Grepping shows there is only one definition, which is the static > function definition in iommu-uclass which is #ifdef'd out when > CONFIG_PCI is disabled. > > ; rg "dev_pci_iommu_enable" > drivers/iommu/iommu-uclass.c > 18:static int dev_pci_iommu_enable(struct udevice *dev) > 116: return dev_pci_iommu_enable(dev); > > > > Am I missing a patch or something? The function is only defined in one > place in the whole of U-Boot. Oh sorry, you're right, I checked the wrong function. I guess I'll defer to Simon on if he prefers to just use #ifdef here or __maybe_unused dev_pci_iommu_enable instead.
On Mon, Dec 11, 2023 at 06:41:40PM +0000, Caleb Connolly wrote: > The dev_pci_iommu_enable() function is only available when CONFIG_PCI is > enabled, replace the runtime check with a preprocessor one to fix > compilation with pci disabled. > > Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org> Applied to u-boot/next, thanks!
diff --git a/drivers/iommu/iommu-uclass.c b/drivers/iommu/iommu-uclass.c index 72f123df55a5..98731d5e2c44 100644 --- a/drivers/iommu/iommu-uclass.c +++ b/drivers/iommu/iommu-uclass.c @@ -100,9 +100,10 @@ int dev_iommu_enable(struct udevice *dev) dev->iommu = dev_iommu; } - if (CONFIG_IS_ENABLED(PCI) && count < 0 && - device_is_on_pci_bus(dev)) +#if CONFIG_IS_ENABLED(PCI) + if (count < 0 && device_is_on_pci_bus(dev)) return dev_pci_iommu_enable(dev); +#endif return 0; }
The dev_pci_iommu_enable() function is only available when CONFIG_PCI is enabled, replace the runtime check with a preprocessor one to fix compilation with pci disabled. Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org> --- drivers/iommu/iommu-uclass.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)