Re: [PATCH v1 2/8] ASoC: Intel: Skylake: Use recently introduced uuid_le_cmp_p{p}()

[Vinod Koul]

On Fri, Apr 21, 2017 at 05:46:39PM +0300, Andy Shevchenko wrote:
> Recently introduced helpers take pointers to uuid_{be|le} instead of
> reference.

Are they in linus's tree, if not it introduces dependency, so we might want
to defer after merge window

> 
> Using them makes code less ugly.

How so..?

> 
> Cc: Liam Girdwood 
> Cc: Mark Brown 
> Cc: Vinod Koul 
> Signed-off-by: Andy Shevchenko 
> ---
>  sound/soc/intel/skylake/skl-pcm.c   | 2 +-
>  sound/soc/intel/skylake/skl-sst-utils.c | 4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/sound/soc/intel/skylake/skl-pcm.c 
> b/sound/soc/intel/skylake/skl-pcm.c
> index 600faad19bd4..4bf171985872 100644
> --- a/sound/soc/intel/skylake/skl-pcm.c
> +++ b/sound/soc/intel/skylake/skl-pcm.c
> @@ -1207,7 +1207,7 @@ static int skl_get_module_info(struct skl *skl, struct 
> skl_module_cfg *mconfig)
>   }
>  
>   list_for_each_entry(module, >uuid_list, list) {
> - if (uuid_le_cmp(*uuid_mod, module->uuid) == 0) {
> + if (uuid_le_cmp_p(uuid_mod, module->uuid) == 0) {
>   mconfig->id.module_id = module->id;
>   mconfig->is_loadable = module->is_loadable;
>   return 0;
> diff --git a/sound/soc/intel/skylake/skl-sst-utils.c 
> b/sound/soc/intel/skylake/skl-sst-utils.c
> index 6d5bff04bf65..67288580c743 100644
> --- a/sound/soc/intel/skylake/skl-sst-utils.c
> +++ b/sound/soc/intel/skylake/skl-sst-utils.c
> @@ -189,7 +189,7 @@ int skl_get_pvt_id(struct skl_sst *ctx, uuid_le 
> *uuid_mod, int instance_id)
>   int pvt_id;
>  
>   list_for_each_entry(module, >uuid_list, list) {
> - if (uuid_le_cmp(*uuid_mod, module->uuid) == 0) {
> + if (uuid_le_cmp_p(uuid_mod, module->uuid) == 0) {
>  
>   pvt_id = skl_pvtid_128(module);
>   if (pvt_id >= 0) {
> @@ -218,7 +218,7 @@ int skl_put_pvt_id(struct skl_sst *ctx, uuid_le 
> *uuid_mod, int *pvt_id)
>   struct uuid_module *module;
>  
>   list_for_each_entry(module, >uuid_list, list) {
> - if (uuid_le_cmp(*uuid_mod, module->uuid) == 0) {
> + if (uuid_le_cmp_p(uuid_mod, module->uuid) == 0) {
>  
>   if (*pvt_id != 0)
>   i = (*pvt_id) / 64;
> -- 
> 2.11.0
> 

-- 
~Vinod
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1 1/8] lib/uuid: Introduce uuid_{be|le}_cmp_p{p}() helpers

[Winkler, Tomas]

On Sun, 2017-04-23 at 15:42 +0300, Andy Shevchenko wrote:
> On Sun, Apr 23, 2017 at 1:29 PM, Winkler, Tomas  com> wrote:
> > > New helpers take pointers to uuid_{be|le} as parameters.
> > > 
> > > When using them on a raw data we don't need to do an ugly
> > > dereference and,
> > > in some cases, a type casting.
> > I think this going overboard, the _pp types  are just enough.
> 
> I looked at existing users and there are cases like
> #define XXX_UUID UUID_...(a, b, c, ...)
> 
> uuid_.*_cmp(value, XXX_UUID)
> 
> For _pp variant if would be _cmp_pp(value, _UUID) which is
> slightly worse than for _p variant.


Maybe it's worth to actually replace the defines with variables than to
create an interface with all the permutations.

Tomas

Re: [PATCH v1 1/8] lib/uuid: Introduce uuid_{be|le}_cmp_p{p}() helpers

[Andy Shevchenko]

On Sun, Apr 23, 2017 at 1:29 PM, Winkler, Tomas  wrote:
>> New helpers take pointers to uuid_{be|le} as parameters.
>>
>> When using them on a raw data we don't need to do an ugly dereference and,
>> in some cases, a type casting.

> I think this going overboard, the _pp types  are just enough.
I looked at existing users and there are cases like
#define XXX_UUID UUID_...(a, b, c, ...)

uuid_.*_cmp(value, XXX_UUID)

For _pp variant if would be _cmp_pp(value, _UUID) which is
slightly worse than for _p variant.

-- 
With Best Regards,
Andy Shevchenko
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] efi/reboot: Fall back to original power-off method if EFI_RESET_SHUTDOWN returns

[Hans de Goede]

Commit 44be28e9dd98 ("x86/reboot: Add EFI reboot quirk for ACPI Hardware
Reduced flag") sets pm_power_off to efi_power_off() when the
acpi_gbl_reduced_hardware flag is set.

According to its commit message this is necessary because: "BayTrail-T
class of hardware requires EFI in order to powerdown and reboot and no
other reliable method exists"

But I have a Bay Trail CR tablet where the EFI_RESET_SHUTDOWN call does
not work, it simply returns without doing anything (AFAICT).

So it seems that some Bay Trail devices must use EFI for power-off, while
for others only ACPI works.

Note that efi_power_off() only gets used if the platform code defines
efi_poweroff_required() and that returns true, this currently only ever
happens on x86.

Since on the devices which need ACPI for power-off the EFI_RESET_SHUTDOWN
call simply returns, this patch makes the efi-reboot code remember the
old pm_power_off handler and if EFI_RESET_SHUTDOWN returns it falls back
to calling that.

This seems preferable to dmi-quirking our way out of this, since there
are likely quite a few devices suffering from this.

Cc: Mark Salter 
Signed-off-by: Hans de Goede 
---
 drivers/firmware/efi/reboot.c | 12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/firmware/efi/reboot.c b/drivers/firmware/efi/reboot.c
index 62ead9b..7117e2d 100644
--- a/drivers/firmware/efi/reboot.c
+++ b/drivers/firmware/efi/reboot.c
@@ -5,6 +5,8 @@
 #include 
 #include 
 
+void (*orig_pm_power_off)(void);
+
 int efi_reboot_quirk_mode = -1;
 
 void efi_reboot(enum reboot_mode reboot_mode, const char *__unused)
@@ -51,6 +53,12 @@ bool __weak efi_poweroff_required(void)
 static void efi_power_off(void)
 {
efi.reset_system(EFI_RESET_SHUTDOWN, EFI_SUCCESS, 0, NULL);
+   /*
+* The above call should not return, if it does fall back to
+* the original power off method (typically ACPI poweroff).
+*/
+   if (orig_pm_power_off)
+   orig_pm_power_off();
 }
 
 static int __init efi_shutdown_init(void)
@@ -58,8 +66,10 @@ static int __init efi_shutdown_init(void)
if (!efi_enabled(EFI_RUNTIME_SERVICES))
return -ENODEV;
 
-   if (efi_poweroff_required())
+   if (efi_poweroff_required()) {
+   orig_pm_power_off = pm_power_off;
pm_power_off = efi_power_off;
+   }
 
return 0;
 }
-- 
2.9.3

--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH v1 1/8] lib/uuid: Introduce uuid_{be|le}_cmp_p{p}() helpers

[Winkler, Tomas]

> New helpers take pointers to uuid_{be|le} as parameters.
> 
> When using them on a raw data we don't need to do an ugly dereference and,
> in some cases, a type casting.
> 
> Cc: Andrew Morton 
> Cc: Arnd Bergmann 
> Cc: Liam Girdwood 
> Cc: Mark Brown 
> Cc: Vinod Koul 
> Cc: Srinivas Pandruvada 
> Cc: Benjamin Tissoires 
> Cc: Kirti Wankhede 
> Cc: Alex Williamson 
> Cc: "K. Y. Srinivasan" 
> Cc: Haiyang Zhang 
> Cc: Stephen Hemminger 
> Cc: Tomas Winkler 
> Cc: Matt Fleming 
> Cc: Ard Biesheuvel 
> Cc: "Rafael J. Wysocki" 
> 
> Signed-off-by: Andy Shevchenko 
> ---
>  include/linux/uuid.h | 20 
>  1 file changed, 20 insertions(+)
> 
> diff --git a/include/linux/uuid.h b/include/linux/uuid.h index
> 4dff73a89758..45312cb5ac65 100644
> --- a/include/linux/uuid.h
> +++ b/include/linux/uuid.h
> @@ -58,6 +58,26 @@ static inline int uuid_be_cmp(const uuid_be u1, const
> uuid_be u2)
>   return memcmp(, , sizeof(uuid_be));  }
> 
> +static inline int uuid_le_cmp_p(const uuid_le *pu1, const uuid_le u2) {
> + return memcmp(pu1, , sizeof(uuid_le)); }
> +
> +static inline int uuid_be_cmp_p(const uuid_be *pu1, const uuid_be u2) {
> + return memcmp(pu1, , sizeof(uuid_be)); }
> +
> +static inline int uuid_le_cmp_pp(const uuid_le *pu1, const uuid_le
> +*pu2) {
> + return memcmp(pu1, pu2, sizeof(uuid_le)); }
> +
> +static inline int uuid_be_cmp_pp(const uuid_be *pu1, const uuid_be
> +*pu2) {
> + return memcmp(pu1, pu2, sizeof(uuid_be)); }
> +
>  void generate_random_uuid(unsigned char uuid[16]);
> 
>  extern void uuid_le_gen(uuid_le *u);

I think this going overboard, the _pp types  are just enough. 
Tomas
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html