Re: [PATCH 05/15] fbdev: imsttfb: remove the dependency on PPC_OF
Hi Kevin, On Tue, 3 Feb 2015 10:20:02 +0800 Kevin Hao wrote: > > I took a second look at this. It seems that there is a declaration of > struct device_node in linux/device.h and there is also no access to the > member of device_node in this driver. So we are safe to not include of.h here. > That is also why I didn't get the build failure for the non-OF archs. :-) Right. Seems ok, then. -- Cheers, Stephen Rothwells...@canb.auug.org.au pgpjJLy0n9AuO.pgp Description: OpenPGP digital signature ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 05/15] fbdev: imsttfb: remove the dependency on PPC_OF
Hi Kevin, On Sun, 1 Feb 2015 13:51:50 +0800 Kevin Hao wrote: > > That was my first thought, but the codes protected by the PPC_OF seem not > ppc specific and should be safe for other archs which also support OF. So I > drop the PPC_OF completely. Did I miss something? Ah, ok. > > > dp = pci_device_to_OF_node(pdev); > > > @@ -1478,7 +1477,6 @@ static int imsttfb_probe(struct pci_dev *pdev, > > > const struct pci_device_id *ent) > > > printk(KERN_INFO "%s: OF name %s\n",__func__, dp->name); > > > else > > > printk(KERN_ERR "imsttfb: no OF node for pci device\n"); > > > -#endif /* CONFIG_PPC_OF */ > > > > This will emit the above error if CONFIG_OF is not set whereas in the > > past it would not. > > How about change it to: > if (IS_ENABLED(CONFIG_OF)) > printk(KERN_ERR "imsttfb: no OF node for pci device\n"); Looks good. -- Cheers, Stephen Rothwells...@canb.auug.org.au pgptJfdFLQlIg.pgp Description: OpenPGP digital signature ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 05/15] fbdev: imsttfb: remove the dependency on PPC_OF
On Sun, Feb 01, 2015 at 01:51:50PM +0800, Kevin Hao wrote: > > > diff --git a/drivers/video/fbdev/imsttfb.c b/drivers/video/fbdev/imsttfb.c > > > index aae10ce74f14..91a80bb8f988 100644 > > > --- a/drivers/video/fbdev/imsttfb.c > > > +++ b/drivers/video/fbdev/imsttfb.c > > > @@ -1470,7 +1470,6 @@ static int imsttfb_probe(struct pci_dev *pdev, > > > const struct pci_device_id *ent) > > > unsigned long addr, size; > > > struct imstt_par *par; > > > struct fb_info *info; > > > -#ifdef CONFIG_PPC_OF > > > struct device_node *dp; > > > > I see no way in this file for struct device_node to be defined > > (especially if CONFIG_PPC is not set). of.h may be included > > implicitly, but that is very dependent on the architecture and CONFIG_ > > options. > > This do pass the build test for the non-OF archs, such as x86. But your > concerns sound pretty reasonable, so I will explicitly include of.h. I took a second look at this. It seems that there is a declaration of struct device_node in linux/device.h and there is also no access to the member of device_node in this driver. So we are safe to not include of.h here. That is also why I didn't get the build failure for the non-OF archs. :-) Thanks, Kevin pgpemgICQ0xRs.pgp Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 05/15] fbdev: imsttfb: remove the dependency on PPC_OF
On Sun, Feb 01, 2015 at 01:44:33PM +1100, Stephen Rothwell wrote: > Hi Kevin, > > On Sat, 31 Jan 2015 21:47:35 +0800 Kevin Hao wrote: > > > > The OF functionality has moved to a common place and be used by many > > archs. So we don't need to depend on PPC_OF option any more. This is > > a preparation for killing PPC_OF. > > I suspect that you want to do the PPC_OF -> PPC conversion on this file > rather than just removing PPC_OF uses. That was my first thought, but the codes protected by the PPC_OF seem not ppc specific and should be safe for other archs which also support OF. So I drop the PPC_OF completely. Did I miss something? > > > diff --git a/drivers/video/fbdev/imsttfb.c b/drivers/video/fbdev/imsttfb.c > > index aae10ce74f14..91a80bb8f988 100644 > > --- a/drivers/video/fbdev/imsttfb.c > > +++ b/drivers/video/fbdev/imsttfb.c > > @@ -1470,7 +1470,6 @@ static int imsttfb_probe(struct pci_dev *pdev, const > > struct pci_device_id *ent) > > unsigned long addr, size; > > struct imstt_par *par; > > struct fb_info *info; > > -#ifdef CONFIG_PPC_OF > > struct device_node *dp; > > I see no way in this file for struct device_node to be defined > (especially if CONFIG_PPC is not set). of.h may be included > implicitly, but that is very dependent on the architecture and CONFIG_ > options. This do pass the build test for the non-OF archs, such as x86. But your concerns sound pretty reasonable, so I will explicitly include of.h. > > > dp = pci_device_to_OF_node(pdev); > > @@ -1478,7 +1477,6 @@ static int imsttfb_probe(struct pci_dev *pdev, const > > struct pci_device_id *ent) > > printk(KERN_INFO "%s: OF name %s\n",__func__, dp->name); > > else > > printk(KERN_ERR "imsttfb: no OF node for pci device\n"); > > -#endif /* CONFIG_PPC_OF */ > > This will emit the above error if CONFIG_OF is not set whereas in the > past it would not. How about change it to: if (IS_ENABLED(CONFIG_OF)) printk(KERN_ERR "imsttfb: no OF node for pci device\n"); Thanks, Kevin pgpCgcltv_MlM.pgp Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 05/15] fbdev: imsttfb: remove the dependency on PPC_OF
Hi Kevin, On Sat, 31 Jan 2015 21:47:35 +0800 Kevin Hao wrote: > > The OF functionality has moved to a common place and be used by many > archs. So we don't need to depend on PPC_OF option any more. This is > a preparation for killing PPC_OF. I suspect that you want to do the PPC_OF -> PPC conversion on this file rather than just removing PPC_OF uses. > diff --git a/drivers/video/fbdev/imsttfb.c b/drivers/video/fbdev/imsttfb.c > index aae10ce74f14..91a80bb8f988 100644 > --- a/drivers/video/fbdev/imsttfb.c > +++ b/drivers/video/fbdev/imsttfb.c > @@ -1470,7 +1470,6 @@ static int imsttfb_probe(struct pci_dev *pdev, const > struct pci_device_id *ent) > unsigned long addr, size; > struct imstt_par *par; > struct fb_info *info; > -#ifdef CONFIG_PPC_OF > struct device_node *dp; I see no way in this file for struct device_node to be defined (especially if CONFIG_PPC is not set). of.h may be included implicitly, but that is very dependent on the architecture and CONFIG_ options. > dp = pci_device_to_OF_node(pdev); > @@ -1478,7 +1477,6 @@ static int imsttfb_probe(struct pci_dev *pdev, const > struct pci_device_id *ent) > printk(KERN_INFO "%s: OF name %s\n",__func__, dp->name); > else > printk(KERN_ERR "imsttfb: no OF node for pci device\n"); > -#endif /* CONFIG_PPC_OF */ This will emit the above error if CONFIG_OF is not set whereas in the past it would not. -- Cheers, Stephen Rothwells...@canb.auug.org.au pgpLCcICwStwh.pgp Description: OpenPGP digital signature ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
