Re: [Linaro-acpi] [RFC PATCH for Juno 1/2] net: smsc911x add support for probing from ACPI
On Mon, Sep 15, 2014 at 05:08:44AM +0100, Grant Likely wrote:
> On Mon, 1 Sep 2014 16:28:54 +0100, Graeme Gregory
> wrote:
> > On Mon, Sep 01, 2014 at 05:17:51PM +0200, Arnd Bergmann wrote:
> > > On Monday 01 September 2014 23:06:00 Hanjun Guo wrote:
> > > > +#ifdef CONFIG_ACPI
> > > > +/* Configure some sensible defaults for ACPI mode */
> > > > +static int smsc911x_probe_config_acpi(struct smsc911x_platform_config
> > > > *config,
> > > > + acpi_handle *ahandle)
> > > > +{
> > > > + if (!ahandle)
> > > > + return -ENOSYS;
> > > > +
> > > > + config->phy_interface = PHY_INTERFACE_MODE_MII;
> > > >
> > >
> > > Please remove the #ifdef and use
> > >
> > > if (!IS_ENABLED(CONFIG_ACPI) || !ahandle)
> > >
> > > to check for ACPI support. This should result in the same object code
> > > in all cases, but give better compile-time coverage when ACPI is
> > > disabled.
> > >
> > struct acpi_handle does not exist in the case !CONFIG_ACPI
> >
> > > Also, -ENOSYS is probably the wrong return value. I think you mean
> > > -ENXIO.
> > >
> > Yes that would make sense thanks.
> >
> > Not sure if we are planning to actually upstream this patch, I guess it
> > depends if ARM start shipping Junos with the ACPI tables loaded on them.
>
> I think we do want this upstreamed. Juno is intended to be a readily
> available reference platform, regardless of the firmware loaded when
> shipped. There will be users who use it as a test platform for ACPI
> development.
The patches for Juno are not intrusive. The only problem I have is
giving the wrong example on how to deal with the platform-specific
device information like this patch. I would much prefer to use _DSD
(once we agree on how to do this) than hard-coding information based on
the ACPI device id (you would need one for each SoC). I don't think we
have reached an agreement yet:
https://lkml.kernel.org/g/4816592.tj3on6vUaC@wuerfel
--
Catalin
--
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: [Linaro-acpi] [RFC PATCH for Juno 1/2] net: smsc911x add support for probing from ACPI
On Mon, Sep 15, 2014 at 05:08:44AM +0100, Grant Likely wrote:
On Mon, 1 Sep 2014 16:28:54 +0100, Graeme Gregory graeme.greg...@linaro.org
wrote:
On Mon, Sep 01, 2014 at 05:17:51PM +0200, Arnd Bergmann wrote:
On Monday 01 September 2014 23:06:00 Hanjun Guo wrote:
+#ifdef CONFIG_ACPI
+/* Configure some sensible defaults for ACPI mode */
+static int smsc911x_probe_config_acpi(struct smsc911x_platform_config
*config,
+ acpi_handle *ahandle)
+{
+ if (!ahandle)
+ return -ENOSYS;
+
+ config-phy_interface = PHY_INTERFACE_MODE_MII;
Please remove the #ifdef and use
if (!IS_ENABLED(CONFIG_ACPI) || !ahandle)
to check for ACPI support. This should result in the same object code
in all cases, but give better compile-time coverage when ACPI is
disabled.
struct acpi_handle does not exist in the case !CONFIG_ACPI
Also, -ENOSYS is probably the wrong return value. I think you mean
-ENXIO.
Yes that would make sense thanks.
Not sure if we are planning to actually upstream this patch, I guess it
depends if ARM start shipping Junos with the ACPI tables loaded on them.
I think we do want this upstreamed. Juno is intended to be a readily
available reference platform, regardless of the firmware loaded when
shipped. There will be users who use it as a test platform for ACPI
development.
The patches for Juno are not intrusive. The only problem I have is
giving the wrong example on how to deal with the platform-specific
device information like this patch. I would much prefer to use _DSD
(once we agree on how to do this) than hard-coding information based on
the ACPI device id (you would need one for each SoC). I don't think we
have reached an agreement yet:
https://lkml.kernel.org/g/4816592.tj3on6vUaC@wuerfel
--
Catalin
--
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: [Linaro-acpi] [RFC PATCH for Juno 1/2] net: smsc911x add support for probing from ACPI
On Mon, 1 Sep 2014 16:28:54 +0100, Graeme Gregory
wrote:
> On Mon, Sep 01, 2014 at 05:17:51PM +0200, Arnd Bergmann wrote:
> > On Monday 01 September 2014 23:06:00 Hanjun Guo wrote:
> > > +#ifdef CONFIG_ACPI
> > > +/* Configure some sensible defaults for ACPI mode */
> > > +static int smsc911x_probe_config_acpi(struct smsc911x_platform_config
> > > *config,
> > > + acpi_handle *ahandle)
> > > +{
> > > + if (!ahandle)
> > > + return -ENOSYS;
> > > +
> > > + config->phy_interface = PHY_INTERFACE_MODE_MII;
> > >
> >
> > Please remove the #ifdef and use
> >
> > if (!IS_ENABLED(CONFIG_ACPI) || !ahandle)
> >
> > to check for ACPI support. This should result in the same object code
> > in all cases, but give better compile-time coverage when ACPI is
> > disabled.
> >
> struct acpi_handle does not exist in the case !CONFIG_ACPI
>
> > Also, -ENOSYS is probably the wrong return value. I think you mean
> > -ENXIO.
> >
> Yes that would make sense thanks.
>
> Not sure if we are planning to actually upstream this patch, I guess it
> depends if ARM start shipping Junos with the ACPI tables loaded on them.
I think we do want this upstreamed. Juno is intended to be a readily
available reference platform, regardless of the firmware loaded when
shipped. There will be users who use it as a test platform for ACPI
development.
g.
--
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: [Linaro-acpi] [RFC PATCH for Juno 1/2] net: smsc911x add support for probing from ACPI
On Mon, 1 Sep 2014 16:28:54 +0100, Graeme Gregory graeme.greg...@linaro.org
wrote:
On Mon, Sep 01, 2014 at 05:17:51PM +0200, Arnd Bergmann wrote:
On Monday 01 September 2014 23:06:00 Hanjun Guo wrote:
+#ifdef CONFIG_ACPI
+/* Configure some sensible defaults for ACPI mode */
+static int smsc911x_probe_config_acpi(struct smsc911x_platform_config
*config,
+ acpi_handle *ahandle)
+{
+ if (!ahandle)
+ return -ENOSYS;
+
+ config-phy_interface = PHY_INTERFACE_MODE_MII;
Please remove the #ifdef and use
if (!IS_ENABLED(CONFIG_ACPI) || !ahandle)
to check for ACPI support. This should result in the same object code
in all cases, but give better compile-time coverage when ACPI is
disabled.
struct acpi_handle does not exist in the case !CONFIG_ACPI
Also, -ENOSYS is probably the wrong return value. I think you mean
-ENXIO.
Yes that would make sense thanks.
Not sure if we are planning to actually upstream this patch, I guess it
depends if ARM start shipping Junos with the ACPI tables loaded on them.
I think we do want this upstreamed. Juno is intended to be a readily
available reference platform, regardless of the firmware loaded when
shipped. There will be users who use it as a test platform for ACPI
development.
g.
--
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: [Linaro-acpi] [RFC PATCH for Juno 1/2] net: smsc911x add support for probing from ACPI
Hi Arnd, On 09/03/2014 11:09 AM, Arnd Bergmann wrote: > On Wednesday 03 September 2014 01:00:23 Rafael J. Wysocki wrote: >> Our intention is specifically not to use "random incompatible bindings" >> in that. We'd rather have a common venue and process for establishing >> new bindings for both DT and _DSD in a compatible way. > > Right, I think everyone is on the same page for the embedded x86 case, > my point was that there is no consensus about that yet among the > parties involved in arm64 servers. However, in the case of MAC devices where you might need to specify just a couple of properties using _DSD, a specific set of discussions has been instigated. As you mentioned, though, this pertains only to certain devices and isn't necessarily true for every Ethernet device. Consequently, while a Juno PoC patch might use a kludge today, that doesn't mean it's a limitation of ACPI, just of certain devices. Jon. -- 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: [Linaro-acpi] [RFC PATCH for Juno 1/2] net: smsc911x add support for probing from ACPI
Hi Arnd, On 09/03/2014 11:09 AM, Arnd Bergmann wrote: On Wednesday 03 September 2014 01:00:23 Rafael J. Wysocki wrote: Our intention is specifically not to use random incompatible bindings in that. We'd rather have a common venue and process for establishing new bindings for both DT and _DSD in a compatible way. Right, I think everyone is on the same page for the embedded x86 case, my point was that there is no consensus about that yet among the parties involved in arm64 servers. However, in the case of MAC devices where you might need to specify just a couple of properties using _DSD, a specific set of discussions has been instigated. As you mentioned, though, this pertains only to certain devices and isn't necessarily true for every Ethernet device. Consequently, while a Juno PoC patch might use a kludge today, that doesn't mean it's a limitation of ACPI, just of certain devices. Jon. -- 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: [Linaro-acpi] [RFC PATCH for Juno 1/2] net: smsc911x add support for probing from ACPI
On Wednesday 03 September 2014 01:00:23 Rafael J. Wysocki wrote: > On Tuesday, September 02, 2014 05:26:06 PM Mark Brown wrote: > > > > --s3puAW9DMBtS2ARW > > Content-Type: text/plain; charset=us-ascii > > Content-Disposition: inline > > > > On Tue, Sep 02, 2014 at 03:42:53PM +0200, Arnd Bergmann wrote: > > > > > The way I recall the discussion, most people were on one extreme > > > side of the discussion or the other: > > > > > a) We should use _DSD for ARM64 servers to maximize code reuse with > > > DT-enabled drivers, work around the slow UEFI standardization process, > > > remain in control of the actual bindings, and avoid the need for > > > endless per-ID platform-data definitions in drivers. > > > > > b) We should never use _DSD at all, since doing that would have no > > > advantage over using DT directly, and we should force every device > > > manufacturer to specify their bindings in an official ACPI document > > > to prevent random incompatible bindings from being established. > > > Any device that shows up in servers should not need arbitrary detailed > > > properties anyway, as the details are supposed to be hidden in AML. > > > > > I can understand the reasons for both approaches, and I find it hard > > > to say either one is invalid. However, the worst possible outcome in > > > my opinion would be having to support a mix of the two. > > > > Right, and the x86 embedded folks are going full steam ahead with _DSD > > regardless so it seems there will be some systems out there using it > > even if they're not ARM servers. > > Our intention is specifically not to use "random incompatible bindings" > in that. We'd rather have a common venue and process for establishing > new bindings for both DT and _DSD in a compatible way. Right, I think everyone is on the same page for the embedded x86 case, my point was that there is no consensus about that yet among the parties involved in arm64 servers. Arnd -- 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: [Linaro-acpi] [RFC PATCH for Juno 1/2] net: smsc911x add support for probing from ACPI
On Wednesday 03 September 2014 01:00:23 Rafael J. Wysocki wrote: On Tuesday, September 02, 2014 05:26:06 PM Mark Brown wrote: --s3puAW9DMBtS2ARW Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Tue, Sep 02, 2014 at 03:42:53PM +0200, Arnd Bergmann wrote: The way I recall the discussion, most people were on one extreme side of the discussion or the other: a) We should use _DSD for ARM64 servers to maximize code reuse with DT-enabled drivers, work around the slow UEFI standardization process, remain in control of the actual bindings, and avoid the need for endless per-ID platform-data definitions in drivers. b) We should never use _DSD at all, since doing that would have no advantage over using DT directly, and we should force every device manufacturer to specify their bindings in an official ACPI document to prevent random incompatible bindings from being established. Any device that shows up in servers should not need arbitrary detailed properties anyway, as the details are supposed to be hidden in AML. I can understand the reasons for both approaches, and I find it hard to say either one is invalid. However, the worst possible outcome in my opinion would be having to support a mix of the two. Right, and the x86 embedded folks are going full steam ahead with _DSD regardless so it seems there will be some systems out there using it even if they're not ARM servers. Our intention is specifically not to use random incompatible bindings in that. We'd rather have a common venue and process for establishing new bindings for both DT and _DSD in a compatible way. Right, I think everyone is on the same page for the embedded x86 case, my point was that there is no consensus about that yet among the parties involved in arm64 servers. Arnd -- 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: [Linaro-acpi] [RFC PATCH for Juno 1/2] net: smsc911x add support for probing from ACPI
On Tuesday, September 02, 2014 05:26:06 PM Mark Brown wrote: > > --s3puAW9DMBtS2ARW > Content-Type: text/plain; charset=us-ascii > Content-Disposition: inline > > On Tue, Sep 02, 2014 at 03:42:53PM +0200, Arnd Bergmann wrote: > > > The way I recall the discussion, most people were on one extreme > > side of the discussion or the other: > > > a) We should use _DSD for ARM64 servers to maximize code reuse with > > DT-enabled drivers, work around the slow UEFI standardization process, > > remain in control of the actual bindings, and avoid the need for > > endless per-ID platform-data definitions in drivers. > > > b) We should never use _DSD at all, since doing that would have no > > advantage over using DT directly, and we should force every device > > manufacturer to specify their bindings in an official ACPI document > > to prevent random incompatible bindings from being established. > > Any device that shows up in servers should not need arbitrary detailed > > properties anyway, as the details are supposed to be hidden in AML. > > > I can understand the reasons for both approaches, and I find it hard > > to say either one is invalid. However, the worst possible outcome in > > my opinion would be having to support a mix of the two. > > Right, and the x86 embedded folks are going full steam ahead with _DSD > regardless so it seems there will be some systems out there using it > even if they're not ARM servers. Our intention is specifically not to use "random incompatible bindings" in that. We'd rather have a common venue and process for establishing new bindings for both DT and _DSD in a compatible way. Rafael -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. -- 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: [Linaro-acpi] [RFC PATCH for Juno 1/2] net: smsc911x add support for probing from ACPI
On Tue, Sep 02, 2014 at 03:42:53PM +0200, Arnd Bergmann wrote: > The way I recall the discussion, most people were on one extreme > side of the discussion or the other: > a) We should use _DSD for ARM64 servers to maximize code reuse with > DT-enabled drivers, work around the slow UEFI standardization process, > remain in control of the actual bindings, and avoid the need for > endless per-ID platform-data definitions in drivers. > b) We should never use _DSD at all, since doing that would have no > advantage over using DT directly, and we should force every device > manufacturer to specify their bindings in an official ACPI document > to prevent random incompatible bindings from being established. > Any device that shows up in servers should not need arbitrary detailed > properties anyway, as the details are supposed to be hidden in AML. > I can understand the reasons for both approaches, and I find it hard > to say either one is invalid. However, the worst possible outcome in > my opinion would be having to support a mix of the two. Right, and the x86 embedded folks are going full steam ahead with _DSD regardless so it seems there will be some systems out there using it even if they're not ARM servers. signature.asc Description: Digital signature
Re: [Linaro-acpi] [RFC PATCH for Juno 1/2] net: smsc911x add support for probing from ACPI
On Tuesday 02 September 2014 14:26:52 Catalin Marinas wrote: > > Not much at the KS, I think it will need to be followed up on lkml > (https://lkml.org/lkml/2014/8/17/10 is the last I'm aware of, not sure > about any updates in the meantime). > > While the above gets sorted, what's the position from an ARM > perspective (and covered by Documentation/arm64/arm-acpi.txt)? I think > the "Device Enumeration" section in this document is fine, it's just the > kernel infrastructure missing. > > Alternatively, you can say _DSD is not allowed (yet?) but I don't > particularly like basing the configuration on acpi_device_id like in > this patch. Which would leave us with ignoring any SoC containing > devices that require such specific configuration. The way I recall the discussion, most people were on one extreme side of the discussion or the other: a) We should use _DSD for ARM64 servers to maximize code reuse with DT-enabled drivers, work around the slow UEFI standardization process, remain in control of the actual bindings, and avoid the need for endless per-ID platform-data definitions in drivers. b) We should never use _DSD at all, since doing that would have no advantage over using DT directly, and we should force every device manufacturer to specify their bindings in an official ACPI document to prevent random incompatible bindings from being established. Any device that shows up in servers should not need arbitrary detailed properties anyway, as the details are supposed to be hidden in AML. I can understand the reasons for both approaches, and I find it hard to say either one is invalid. However, the worst possible outcome in my opinion would be having to support a mix of the two. Arnd -- 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: [Linaro-acpi] [RFC PATCH for Juno 1/2] net: smsc911x add support for probing from ACPI
On Tuesday 02 September 2014 14:26:52 Catalin Marinas wrote: Not much at the KS, I think it will need to be followed up on lkml (https://lkml.org/lkml/2014/8/17/10 is the last I'm aware of, not sure about any updates in the meantime). While the above gets sorted, what's the position from an ARM perspective (and covered by Documentation/arm64/arm-acpi.txt)? I think the Device Enumeration section in this document is fine, it's just the kernel infrastructure missing. Alternatively, you can say _DSD is not allowed (yet?) but I don't particularly like basing the configuration on acpi_device_id like in this patch. Which would leave us with ignoring any SoC containing devices that require such specific configuration. The way I recall the discussion, most people were on one extreme side of the discussion or the other: a) We should use _DSD for ARM64 servers to maximize code reuse with DT-enabled drivers, work around the slow UEFI standardization process, remain in control of the actual bindings, and avoid the need for endless per-ID platform-data definitions in drivers. b) We should never use _DSD at all, since doing that would have no advantage over using DT directly, and we should force every device manufacturer to specify their bindings in an official ACPI document to prevent random incompatible bindings from being established. Any device that shows up in servers should not need arbitrary detailed properties anyway, as the details are supposed to be hidden in AML. I can understand the reasons for both approaches, and I find it hard to say either one is invalid. However, the worst possible outcome in my opinion would be having to support a mix of the two. Arnd -- 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: [Linaro-acpi] [RFC PATCH for Juno 1/2] net: smsc911x add support for probing from ACPI
On Tue, Sep 02, 2014 at 03:42:53PM +0200, Arnd Bergmann wrote: The way I recall the discussion, most people were on one extreme side of the discussion or the other: a) We should use _DSD for ARM64 servers to maximize code reuse with DT-enabled drivers, work around the slow UEFI standardization process, remain in control of the actual bindings, and avoid the need for endless per-ID platform-data definitions in drivers. b) We should never use _DSD at all, since doing that would have no advantage over using DT directly, and we should force every device manufacturer to specify their bindings in an official ACPI document to prevent random incompatible bindings from being established. Any device that shows up in servers should not need arbitrary detailed properties anyway, as the details are supposed to be hidden in AML. I can understand the reasons for both approaches, and I find it hard to say either one is invalid. However, the worst possible outcome in my opinion would be having to support a mix of the two. Right, and the x86 embedded folks are going full steam ahead with _DSD regardless so it seems there will be some systems out there using it even if they're not ARM servers. signature.asc Description: Digital signature
Re: [Linaro-acpi] [RFC PATCH for Juno 1/2] net: smsc911x add support for probing from ACPI
On Tuesday, September 02, 2014 05:26:06 PM Mark Brown wrote: --s3puAW9DMBtS2ARW Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Tue, Sep 02, 2014 at 03:42:53PM +0200, Arnd Bergmann wrote: The way I recall the discussion, most people were on one extreme side of the discussion or the other: a) We should use _DSD for ARM64 servers to maximize code reuse with DT-enabled drivers, work around the slow UEFI standardization process, remain in control of the actual bindings, and avoid the need for endless per-ID platform-data definitions in drivers. b) We should never use _DSD at all, since doing that would have no advantage over using DT directly, and we should force every device manufacturer to specify their bindings in an official ACPI document to prevent random incompatible bindings from being established. Any device that shows up in servers should not need arbitrary detailed properties anyway, as the details are supposed to be hidden in AML. I can understand the reasons for both approaches, and I find it hard to say either one is invalid. However, the worst possible outcome in my opinion would be having to support a mix of the two. Right, and the x86 embedded folks are going full steam ahead with _DSD regardless so it seems there will be some systems out there using it even if they're not ARM servers. Our intention is specifically not to use random incompatible bindings in that. We'd rather have a common venue and process for establishing new bindings for both DT and _DSD in a compatible way. Rafael -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. -- 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: [Linaro-acpi] [RFC PATCH for Juno 1/2] net: smsc911x add support for probing from ACPI
On 01/09/14 18:14, Catalin Marinas wrote: On Mon, Sep 01, 2014 at 06:08:49PM +0100, Sudeep Holla wrote: On 01/09/14 17:58, Mark Brown wrote: On Mon, Sep 01, 2014 at 05:53:33PM +0100, Catalin Marinas wrote: Confused. Then how come smsc911x_drv_probe() has this line: acpi_handle *ahandle = ACPI_HANDLE(>dev); without any #ifdef's. There's a stub smsc911x_probe_config_acpi() provided in the non-ACPI case, Arnd's suggestion is basically to remove the stub. I think Catalin is referring to ACPI_HANDLE used without any #ifdefs Catalin, few macros like ACPI_HANDLE and ACPI_PTR are defined in include/linux/acpi.h even when CONFIG_ACPI is not set mainly to avoid #ifdef's around simple assignments like the above one and one in platform_driver.acpi_match_table My comment was to Graeme who said that #ifdef's were needed because acpi_handle (lowercase) was not defined in the !CONFIG_ACPI case. However, further down in the patch it was used without any #ifdef's. Ah OK, I misunderstood. In that case Graeme statement is wrong. IIRC acpi_handle is not structure, it's just a ptr used for object references in ACPI namespace and is available even when !CONFIG_ACPI Regards, Sudeep -- 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: [Linaro-acpi] [RFC PATCH for Juno 1/2] net: smsc911x add support for probing from ACPI
On Mon, Sep 01, 2014 at 06:08:49PM +0100, Sudeep Holla wrote: > On 01/09/14 17:58, Mark Brown wrote: > > On Mon, Sep 01, 2014 at 05:53:33PM +0100, Catalin Marinas wrote: > > > >> Confused. Then how come smsc911x_drv_probe() has this line: > > > >>acpi_handle *ahandle = ACPI_HANDLE(>dev); > > > >> without any #ifdef's. > > > > There's a stub smsc911x_probe_config_acpi() provided in the non-ACPI > > case, Arnd's suggestion is basically to remove the stub. > > > > I think Catalin is referring to ACPI_HANDLE used without any #ifdefs > > Catalin, few macros like ACPI_HANDLE and ACPI_PTR are defined in > include/linux/acpi.h even when CONFIG_ACPI is not set mainly to > avoid #ifdef's around simple assignments like the above one and one > in platform_driver.acpi_match_table My comment was to Graeme who said that #ifdef's were needed because acpi_handle (lowercase) was not defined in the !CONFIG_ACPI case. However, further down in the patch it was used without any #ifdef's. -- Catalin -- 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: [Linaro-acpi] [RFC PATCH for Juno 1/2] net: smsc911x add support for probing from ACPI
Hi Mark, On 01/09/14 17:58, Mark Brown wrote: On Mon, Sep 01, 2014 at 05:53:33PM +0100, Catalin Marinas wrote: Confused. Then how come smsc911x_drv_probe() has this line: acpi_handle *ahandle = ACPI_HANDLE(>dev); without any #ifdef's. There's a stub smsc911x_probe_config_acpi() provided in the non-ACPI case, Arnd's suggestion is basically to remove the stub. I think Catalin is referring to ACPI_HANDLE used without any #ifdefs Catalin, few macros like ACPI_HANDLE and ACPI_PTR are defined in include/linux/acpi.h even when CONFIG_ACPI is not set mainly to avoid #ifdef's around simple assignments like the above one and one in platform_driver.acpi_match_table Regards, Sudeep -- 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: [Linaro-acpi] [RFC PATCH for Juno 1/2] net: smsc911x add support for probing from ACPI
On Mon, Sep 01, 2014 at 05:53:33PM +0100, Catalin Marinas wrote:
> On Mon, Sep 01, 2014 at 04:28:54PM +0100, Graeme Gregory wrote:
> > On Mon, Sep 01, 2014 at 05:17:51PM +0200, Arnd Bergmann wrote:
> > > On Monday 01 September 2014 23:06:00 Hanjun Guo wrote:
> > > > +#ifdef CONFIG_ACPI
> > > > +/* Configure some sensible defaults for ACPI mode */
> > > > +static int smsc911x_probe_config_acpi(struct smsc911x_platform_config
> > > > *config,
> > > > + acpi_handle *ahandle)
> > > > +{
> > > > + if (!ahandle)
> > > > + return -ENOSYS;
> > > > +
> > > > + config->phy_interface = PHY_INTERFACE_MODE_MII;
> > > >
> > >
> > > Please remove the #ifdef and use
> > >
> > > if (!IS_ENABLED(CONFIG_ACPI) || !ahandle)
> > >
> > > to check for ACPI support. This should result in the same object code
> > > in all cases, but give better compile-time coverage when ACPI is
> > > disabled.
> > >
> > struct acpi_handle does not exist in the case !CONFIG_ACPI
>
> Confused. Then how come smsc911x_drv_probe() has this line:
>
> acpi_handle *ahandle = ACPI_HANDLE(>dev);
>
> without any #ifdef's.
>
It is possible I confused myself as some types moved around in ACPI
recently. I will re-check!
Graeme
--
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: [Linaro-acpi] [RFC PATCH for Juno 1/2] net: smsc911x add support for probing from ACPI
On Mon, Sep 01, 2014 at 05:53:33PM +0100, Catalin Marinas wrote: > Confused. Then how come smsc911x_drv_probe() has this line: > acpi_handle *ahandle = ACPI_HANDLE(>dev); > without any #ifdef's. There's a stub smsc911x_probe_config_acpi() provided in the non-ACPI case, Arnd's suggestion is basically to remove the stub. signature.asc Description: Digital signature
Re: [Linaro-acpi] [RFC PATCH for Juno 1/2] net: smsc911x add support for probing from ACPI
On Mon, Sep 01, 2014 at 04:28:54PM +0100, Graeme Gregory wrote:
> On Mon, Sep 01, 2014 at 05:17:51PM +0200, Arnd Bergmann wrote:
> > On Monday 01 September 2014 23:06:00 Hanjun Guo wrote:
> > > +#ifdef CONFIG_ACPI
> > > +/* Configure some sensible defaults for ACPI mode */
> > > +static int smsc911x_probe_config_acpi(struct smsc911x_platform_config
> > > *config,
> > > + acpi_handle *ahandle)
> > > +{
> > > + if (!ahandle)
> > > + return -ENOSYS;
> > > +
> > > + config->phy_interface = PHY_INTERFACE_MODE_MII;
> > >
> >
> > Please remove the #ifdef and use
> >
> > if (!IS_ENABLED(CONFIG_ACPI) || !ahandle)
> >
> > to check for ACPI support. This should result in the same object code
> > in all cases, but give better compile-time coverage when ACPI is
> > disabled.
> >
> struct acpi_handle does not exist in the case !CONFIG_ACPI
Confused. Then how come smsc911x_drv_probe() has this line:
acpi_handle *ahandle = ACPI_HANDLE(>dev);
without any #ifdef's.
--
Catalin
--
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: [Linaro-acpi] [RFC PATCH for Juno 1/2] net: smsc911x add support for probing from ACPI
On Monday 01 September 2014 16:28:54 Graeme Gregory wrote:
> On Mon, Sep 01, 2014 at 05:17:51PM +0200, Arnd Bergmann wrote:
> > On Monday 01 September 2014 23:06:00 Hanjun Guo wrote:
> > > +#ifdef CONFIG_ACPI
> > > +/* Configure some sensible defaults for ACPI mode */
> > > +static int smsc911x_probe_config_acpi(struct smsc911x_platform_config
> > > *config,
> > > + acpi_handle *ahandle)
> > > +{
> > > + if (!ahandle)
> > > + return -ENOSYS;
> > > +
> > > + config->phy_interface = PHY_INTERFACE_MODE_MII;
> > >
> >
> > Please remove the #ifdef and use
> >
> > if (!IS_ENABLED(CONFIG_ACPI) || !ahandle)
> >
> > to check for ACPI support. This should result in the same object code
> > in all cases, but give better compile-time coverage when ACPI is
> > disabled.
> >
> struct acpi_handle does not exist in the case !CONFIG_ACPI
That should be easy to fix, and a good idea in general, independent
of this patch.
We generally make function declarations and type definitions visible
(possibly empty) for disabled subsystems so the code using them
silently goes away when that subsystem is disabled.
Arnd
--
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: [Linaro-acpi] [RFC PATCH for Juno 1/2] net: smsc911x add support for probing from ACPI
On Mon, Sep 01, 2014 at 05:17:51PM +0200, Arnd Bergmann wrote:
> On Monday 01 September 2014 23:06:00 Hanjun Guo wrote:
> > +#ifdef CONFIG_ACPI
> > +/* Configure some sensible defaults for ACPI mode */
> > +static int smsc911x_probe_config_acpi(struct smsc911x_platform_config
> > *config,
> > + acpi_handle *ahandle)
> > +{
> > + if (!ahandle)
> > + return -ENOSYS;
> > +
> > + config->phy_interface = PHY_INTERFACE_MODE_MII;
> >
>
> Please remove the #ifdef and use
>
> if (!IS_ENABLED(CONFIG_ACPI) || !ahandle)
>
> to check for ACPI support. This should result in the same object code
> in all cases, but give better compile-time coverage when ACPI is
> disabled.
>
struct acpi_handle does not exist in the case !CONFIG_ACPI
> Also, -ENOSYS is probably the wrong return value. I think you mean
> -ENXIO.
>
Yes that would make sense thanks.
Not sure if we are planning to actually upstream this patch, I guess it
depends if ARM start shipping Junos with the ACPI tables loaded on them.
Graeme
--
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: [Linaro-acpi] [RFC PATCH for Juno 1/2] net: smsc911x add support for probing from ACPI
On Monday 01 September 2014 23:06:00 Hanjun Guo wrote:
> +#ifdef CONFIG_ACPI
> +/* Configure some sensible defaults for ACPI mode */
> +static int smsc911x_probe_config_acpi(struct smsc911x_platform_config
> *config,
> + acpi_handle *ahandle)
> +{
> + if (!ahandle)
> + return -ENOSYS;
> +
> + config->phy_interface = PHY_INTERFACE_MODE_MII;
>
Please remove the #ifdef and use
if (!IS_ENABLED(CONFIG_ACPI) || !ahandle)
to check for ACPI support. This should result in the same object code
in all cases, but give better compile-time coverage when ACPI is
disabled.
Also, -ENOSYS is probably the wrong return value. I think you mean
-ENXIO.
Arnd
--
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: [Linaro-acpi] [RFC PATCH for Juno 1/2] net: smsc911x add support for probing from ACPI
On Monday 01 September 2014 23:06:00 Hanjun Guo wrote:
+#ifdef CONFIG_ACPI
+/* Configure some sensible defaults for ACPI mode */
+static int smsc911x_probe_config_acpi(struct smsc911x_platform_config
*config,
+ acpi_handle *ahandle)
+{
+ if (!ahandle)
+ return -ENOSYS;
+
+ config-phy_interface = PHY_INTERFACE_MODE_MII;
Please remove the #ifdef and use
if (!IS_ENABLED(CONFIG_ACPI) || !ahandle)
to check for ACPI support. This should result in the same object code
in all cases, but give better compile-time coverage when ACPI is
disabled.
Also, -ENOSYS is probably the wrong return value. I think you mean
-ENXIO.
Arnd
--
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: [Linaro-acpi] [RFC PATCH for Juno 1/2] net: smsc911x add support for probing from ACPI
On Mon, Sep 01, 2014 at 05:17:51PM +0200, Arnd Bergmann wrote:
On Monday 01 September 2014 23:06:00 Hanjun Guo wrote:
+#ifdef CONFIG_ACPI
+/* Configure some sensible defaults for ACPI mode */
+static int smsc911x_probe_config_acpi(struct smsc911x_platform_config
*config,
+ acpi_handle *ahandle)
+{
+ if (!ahandle)
+ return -ENOSYS;
+
+ config-phy_interface = PHY_INTERFACE_MODE_MII;
Please remove the #ifdef and use
if (!IS_ENABLED(CONFIG_ACPI) || !ahandle)
to check for ACPI support. This should result in the same object code
in all cases, but give better compile-time coverage when ACPI is
disabled.
struct acpi_handle does not exist in the case !CONFIG_ACPI
Also, -ENOSYS is probably the wrong return value. I think you mean
-ENXIO.
Yes that would make sense thanks.
Not sure if we are planning to actually upstream this patch, I guess it
depends if ARM start shipping Junos with the ACPI tables loaded on them.
Graeme
--
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: [Linaro-acpi] [RFC PATCH for Juno 1/2] net: smsc911x add support for probing from ACPI
On Monday 01 September 2014 16:28:54 Graeme Gregory wrote:
On Mon, Sep 01, 2014 at 05:17:51PM +0200, Arnd Bergmann wrote:
On Monday 01 September 2014 23:06:00 Hanjun Guo wrote:
+#ifdef CONFIG_ACPI
+/* Configure some sensible defaults for ACPI mode */
+static int smsc911x_probe_config_acpi(struct smsc911x_platform_config
*config,
+ acpi_handle *ahandle)
+{
+ if (!ahandle)
+ return -ENOSYS;
+
+ config-phy_interface = PHY_INTERFACE_MODE_MII;
Please remove the #ifdef and use
if (!IS_ENABLED(CONFIG_ACPI) || !ahandle)
to check for ACPI support. This should result in the same object code
in all cases, but give better compile-time coverage when ACPI is
disabled.
struct acpi_handle does not exist in the case !CONFIG_ACPI
That should be easy to fix, and a good idea in general, independent
of this patch.
We generally make function declarations and type definitions visible
(possibly empty) for disabled subsystems so the code using them
silently goes away when that subsystem is disabled.
Arnd
--
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: [Linaro-acpi] [RFC PATCH for Juno 1/2] net: smsc911x add support for probing from ACPI
On Mon, Sep 01, 2014 at 04:28:54PM +0100, Graeme Gregory wrote:
On Mon, Sep 01, 2014 at 05:17:51PM +0200, Arnd Bergmann wrote:
On Monday 01 September 2014 23:06:00 Hanjun Guo wrote:
+#ifdef CONFIG_ACPI
+/* Configure some sensible defaults for ACPI mode */
+static int smsc911x_probe_config_acpi(struct smsc911x_platform_config
*config,
+ acpi_handle *ahandle)
+{
+ if (!ahandle)
+ return -ENOSYS;
+
+ config-phy_interface = PHY_INTERFACE_MODE_MII;
Please remove the #ifdef and use
if (!IS_ENABLED(CONFIG_ACPI) || !ahandle)
to check for ACPI support. This should result in the same object code
in all cases, but give better compile-time coverage when ACPI is
disabled.
struct acpi_handle does not exist in the case !CONFIG_ACPI
Confused. Then how come smsc911x_drv_probe() has this line:
acpi_handle *ahandle = ACPI_HANDLE(pdev-dev);
without any #ifdef's.
--
Catalin
--
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: [Linaro-acpi] [RFC PATCH for Juno 1/2] net: smsc911x add support for probing from ACPI
On Mon, Sep 01, 2014 at 05:53:33PM +0100, Catalin Marinas wrote: Confused. Then how come smsc911x_drv_probe() has this line: acpi_handle *ahandle = ACPI_HANDLE(pdev-dev); without any #ifdef's. There's a stub smsc911x_probe_config_acpi() provided in the non-ACPI case, Arnd's suggestion is basically to remove the stub. signature.asc Description: Digital signature
Re: [Linaro-acpi] [RFC PATCH for Juno 1/2] net: smsc911x add support for probing from ACPI
On Mon, Sep 01, 2014 at 05:53:33PM +0100, Catalin Marinas wrote:
On Mon, Sep 01, 2014 at 04:28:54PM +0100, Graeme Gregory wrote:
On Mon, Sep 01, 2014 at 05:17:51PM +0200, Arnd Bergmann wrote:
On Monday 01 September 2014 23:06:00 Hanjun Guo wrote:
+#ifdef CONFIG_ACPI
+/* Configure some sensible defaults for ACPI mode */
+static int smsc911x_probe_config_acpi(struct smsc911x_platform_config
*config,
+ acpi_handle *ahandle)
+{
+ if (!ahandle)
+ return -ENOSYS;
+
+ config-phy_interface = PHY_INTERFACE_MODE_MII;
Please remove the #ifdef and use
if (!IS_ENABLED(CONFIG_ACPI) || !ahandle)
to check for ACPI support. This should result in the same object code
in all cases, but give better compile-time coverage when ACPI is
disabled.
struct acpi_handle does not exist in the case !CONFIG_ACPI
Confused. Then how come smsc911x_drv_probe() has this line:
acpi_handle *ahandle = ACPI_HANDLE(pdev-dev);
without any #ifdef's.
It is possible I confused myself as some types moved around in ACPI
recently. I will re-check!
Graeme
--
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: [Linaro-acpi] [RFC PATCH for Juno 1/2] net: smsc911x add support for probing from ACPI
Hi Mark, On 01/09/14 17:58, Mark Brown wrote: On Mon, Sep 01, 2014 at 05:53:33PM +0100, Catalin Marinas wrote: Confused. Then how come smsc911x_drv_probe() has this line: acpi_handle *ahandle = ACPI_HANDLE(pdev-dev); without any #ifdef's. There's a stub smsc911x_probe_config_acpi() provided in the non-ACPI case, Arnd's suggestion is basically to remove the stub. I think Catalin is referring to ACPI_HANDLE used without any #ifdefs Catalin, few macros like ACPI_HANDLE and ACPI_PTR are defined in include/linux/acpi.h even when CONFIG_ACPI is not set mainly to avoid #ifdef's around simple assignments like the above one and one in platform_driver.acpi_match_table Regards, Sudeep -- 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: [Linaro-acpi] [RFC PATCH for Juno 1/2] net: smsc911x add support for probing from ACPI
On Mon, Sep 01, 2014 at 06:08:49PM +0100, Sudeep Holla wrote: On 01/09/14 17:58, Mark Brown wrote: On Mon, Sep 01, 2014 at 05:53:33PM +0100, Catalin Marinas wrote: Confused. Then how come smsc911x_drv_probe() has this line: acpi_handle *ahandle = ACPI_HANDLE(pdev-dev); without any #ifdef's. There's a stub smsc911x_probe_config_acpi() provided in the non-ACPI case, Arnd's suggestion is basically to remove the stub. I think Catalin is referring to ACPI_HANDLE used without any #ifdefs Catalin, few macros like ACPI_HANDLE and ACPI_PTR are defined in include/linux/acpi.h even when CONFIG_ACPI is not set mainly to avoid #ifdef's around simple assignments like the above one and one in platform_driver.acpi_match_table My comment was to Graeme who said that #ifdef's were needed because acpi_handle (lowercase) was not defined in the !CONFIG_ACPI case. However, further down in the patch it was used without any #ifdef's. -- Catalin -- 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: [Linaro-acpi] [RFC PATCH for Juno 1/2] net: smsc911x add support for probing from ACPI
On 01/09/14 18:14, Catalin Marinas wrote: On Mon, Sep 01, 2014 at 06:08:49PM +0100, Sudeep Holla wrote: On 01/09/14 17:58, Mark Brown wrote: On Mon, Sep 01, 2014 at 05:53:33PM +0100, Catalin Marinas wrote: Confused. Then how come smsc911x_drv_probe() has this line: acpi_handle *ahandle = ACPI_HANDLE(pdev-dev); without any #ifdef's. There's a stub smsc911x_probe_config_acpi() provided in the non-ACPI case, Arnd's suggestion is basically to remove the stub. I think Catalin is referring to ACPI_HANDLE used without any #ifdefs Catalin, few macros like ACPI_HANDLE and ACPI_PTR are defined in include/linux/acpi.h even when CONFIG_ACPI is not set mainly to avoid #ifdef's around simple assignments like the above one and one in platform_driver.acpi_match_table My comment was to Graeme who said that #ifdef's were needed because acpi_handle (lowercase) was not defined in the !CONFIG_ACPI case. However, further down in the patch it was used without any #ifdef's. Ah OK, I misunderstood. In that case Graeme statement is wrong. IIRC acpi_handle is not structure, it's just a ptr used for object references in ACPI namespace and is available even when !CONFIG_ACPI Regards, Sudeep -- 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/
