Re: [RFC 1/2] i2c: Use stable dev_name for ACPI enumerated I2C slaves
On Friday, November 01, 2013 01:08:47 AM Rafael J. Wysocki wrote:
> On Monday, October 28, 2013 03:15:25 PM Jarkko Nikula wrote:
> > Hi Rafael
> >
> > On 10/25/2013 05:08 PM, Rafael J. Wysocki wrote:
> > > On Friday, October 25, 2013 04:30:23 PM Jarkko Nikula wrote:
> > >>>
> > >> Hmm, not only. Referencing to dev field in struct acpi_device by
> > >> dev_name(&adev->dev) here will fail too.
> > >
> > > Well, something is not quite right here.
> > >
> > > One of the *reasons* for having ACPI_HANDLE() defined this way is to
> > > avoid using explicit CONFIG_ACPI checks, so if that doesn't work,
> > > then all of that becomes a bit pointless.
> > >
>
> Can you please send patches inline instead of using inline attachments,
> so that people don't have to paste your patches back to comment them?
>
> > One possible thing to do is to let structure definitions to be available
> > for non-ACPI builds. Then compiler won't fail on structure access which
> > will be anyway optimized away by later compiler stages.
>
> Yes, we can do that, but as I said that means we're giving up on the "why
> ACPI_HANDLE() doesn't work as expected" front. For now, I'd like to
> understand what's up before making that change. Moreover ->
OK, so I *think* what happens is that the compiler checks if the particular
symbols make sense before trying to optimize out the whole block.
So the patch below modulo the return value of the acpi_bus_get_device() stub
would be fine by me.
Thanks!
> > With a quick test below vmlinux section sizes don't change for couple
> > non-ACPI build tests and allow to get rid off IS_ENABLED(CONFIG_ACPI)
> > test in this patch. It's very minimal as it only moves the CONFIG_ACPI
> > test just after the struct acpi_device definition and defines
> > acpi_bus_get_device for non-ACPI case.
> >
> > What I don't know how consistent it is as there are still couple
> > structure definitions under CONFIG_ACPI, doesn't touch other acpi
> > headers and requires to include acpi_bus.h in driver (or move acpi_bus.h
> > include in linux/acpi.h currently under CONFIG_ACPI).
> >
> > diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
> > index d901982..7ab7870 100644
> > --- a/include/acpi/acpi_bus.h
> > +++ b/include/acpi/acpi_bus.h
> > @@ -66,8 +66,6 @@ bool acpi_ata_match(acpi_handle handle);
> >
> > bool acpi_bay_match(acpi_handle handle);
> > bool acpi_dock_match(acpi_handle handle);
> >
> > -#ifdef CONFIG_ACPI
> > -
> >
> > #include
> >
> > #define ACPI_BUS_FILE_ROOT"acpi"
> >
> > @@ -314,6 +312,8 @@ struct acpi_device {
> >
> > void (*remove)(struct acpi_device *);
> >
> > };
> >
> > +#if IS_ENABLED(CONFIG_ACPI)
> > +
> >
> > static inline void *acpi_driver_data(struct acpi_device *d)
> > {
> >
> > return d->driver_data;
> >
> > @@ -531,6 +531,8 @@ static inline bool acpi_device_can_poweroff(struct
> > acpi_device *adev)
> >
> > static inline int register_acpi_bus_type(void *bus) { return 0; }
> > static inline int unregister_acpi_bus_type(void *bus) { return 0; }
> >
> > +static inline int acpi_bus_get_device(acpi_handle handle,
> > + struct acpi_device **device) { return 0; }
>
> This has to return an error code, because otherwise the caller can assume
> *device to be a valid pointer after it returns which may not be the case.
>
> >
> > #endif/* CONFIG_ACPI */
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC 1/2] i2c: Use stable dev_name for ACPI enumerated I2C slaves
On Monday, October 28, 2013 03:15:25 PM Jarkko Nikula wrote:
> Hi Rafael
>
> On 10/25/2013 05:08 PM, Rafael J. Wysocki wrote:
> > On Friday, October 25, 2013 04:30:23 PM Jarkko Nikula wrote:
> >>>
> >> Hmm, not only. Referencing to dev field in struct acpi_device by
> >> dev_name(&adev->dev) here will fail too.
> >
> > Well, something is not quite right here.
> >
> > One of the *reasons* for having ACPI_HANDLE() defined this way is to
> > avoid using explicit CONFIG_ACPI checks, so if that doesn't work,
> > then all of that becomes a bit pointless.
> >
Can you please send patches inline instead of using inline attachments,
so that people don't have to paste your patches back to comment them?
> One possible thing to do is to let structure definitions to be available
> for non-ACPI builds. Then compiler won't fail on structure access which
> will be anyway optimized away by later compiler stages.
Yes, we can do that, but as I said that means we're giving up on the "why
ACPI_HANDLE() doesn't work as expected" front. For now, I'd like to
understand what's up before making that change. Moreover ->
> With a quick test below vmlinux section sizes don't change for couple
> non-ACPI build tests and allow to get rid off IS_ENABLED(CONFIG_ACPI)
> test in this patch. It's very minimal as it only moves the CONFIG_ACPI
> test just after the struct acpi_device definition and defines
> acpi_bus_get_device for non-ACPI case.
>
> What I don't know how consistent it is as there are still couple
> structure definitions under CONFIG_ACPI, doesn't touch other acpi
> headers and requires to include acpi_bus.h in driver (or move acpi_bus.h
> include in linux/acpi.h currently under CONFIG_ACPI).
>
> diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
> index d901982..7ab7870 100644
> --- a/include/acpi/acpi_bus.h
> +++ b/include/acpi/acpi_bus.h
> @@ -66,8 +66,6 @@ bool acpi_ata_match(acpi_handle handle);
>
> bool acpi_bay_match(acpi_handle handle);
> bool acpi_dock_match(acpi_handle handle);
>
> -#ifdef CONFIG_ACPI
> -
>
> #include
>
> #define ACPI_BUS_FILE_ROOT"acpi"
>
> @@ -314,6 +312,8 @@ struct acpi_device {
>
> void (*remove)(struct acpi_device *);
>
> };
>
> +#if IS_ENABLED(CONFIG_ACPI)
> +
>
> static inline void *acpi_driver_data(struct acpi_device *d)
> {
>
> return d->driver_data;
>
> @@ -531,6 +531,8 @@ static inline bool acpi_device_can_poweroff(struct
> acpi_device *adev)
>
> static inline int register_acpi_bus_type(void *bus) { return 0; }
> static inline int unregister_acpi_bus_type(void *bus) { return 0; }
>
> +static inline int acpi_bus_get_device(acpi_handle handle,
> + struct acpi_device **device) { return 0; }
This has to return an error code, because otherwise the caller can assume
*device to be a valid pointer after it returns which may not be the case.
>
> #endif/* CONFIG_ACPI */
Thanks!
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] i2c: rcar: fixup rcar type naming
On Thu, Oct 03, 2013 at 11:36:52PM -0700, Kuninori Morimoto wrote: > b720423a2627f045133bec39a31fe2bc0dab86f3 > (i2c: rcar: add rcar-H2 support) > added R-Car H2 support on i2c-rcar. > But the added i2c type naming was H1/H2, > instead of Gen1/Gen2 (Generation 1/2) > Gen1/Gen2 is better naming on this driver. > > This patch exchanges rcar_i2c_id_table[], > but it still can keep compatible, since still there is no user > for i2c-rcar_h1/h2 at this point. > > Signed-off-by: Kuninori Morimoto OK, since the driver is relatively new: Applied to for-next, thanks! signature.asc Description: Digital signature
Re: [patch] i2c-scmi: remove some bogus NULL checks
On Sat, Oct 19, 2013 at 11:46:22AM +0300, Dan Carpenter wrote: > "obj" can't be NULL here. > > We already know that "pkg->package.elements" gives us a valid pointer > so the next pointer after that is also non-NULL. > > Signed-off-by: Dan Carpenter Applied to for-next, thanks! signature.asc Description: Digital signature
Re: [PATCH v2 03/19] i2c: sh_mobile: Enable the driver on all ARM platforms
On Tue, Oct 29, 2013 at 11:37:38PM +0100, Laurent Pinchart wrote: > Renesas ARM platforms are transitioning from single-platform to > multi-platform kernels using the new ARCH_SHMOBILE_MULTI. Make the > driver available on all ARM platforms to enable it on both ARCH_SHMOBILE > and ARCH_SHMOBILE_MULTI, and increase build testing coverage with > COMPILE_TEST. > > Cc: Wolfram Sang > Cc: linux-i2c@vger.kernel.org > Signed-off-by: Laurent Pinchart > Acked-by: Wolfram Sang Both i2c patches squashed and applied to for-next, thanks! signature.asc Description: Digital signature
Re: [PATCH 06/12] i2c: sh_mobile: Convert to clk_prepare/unprepare
On Mon, Oct 28, 2013 at 11:49:23PM +0100, Laurent Pinchart wrote: > Turn clk_enable() and clk_disable() calls into clk_prepare_enable() and > clk_disable_unprepare() to get ready for the migration to the common > clock framework. > > Cc: Wolfram Sang > Cc: linux-i2c@vger.kernel.org > Signed-off-by: Laurent Pinchart Applied to for-next, thanks! signature.asc Description: Digital signature
