Re: [Nouveau] [PATCH v1] ACPI: Switch to use generic UUID API

2017-05-15 Thread Mathias Nyman



diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index 7b86508ac8cf..93b4f0de9418 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -210,13 +210,12 @@ static void xhci_pci_quirks(struct device *dev, struct 
xhci_hcd *xhci)
  #ifdef CONFIG_ACPI
  static void xhci_pme_acpi_rtd3_enable(struct pci_dev *dev)
  {
-   static const u8 intel_dsm_uuid[] = {
-   0xb7, 0x0c, 0x34, 0xac, 0x01, 0xe9, 0xbf, 0x45,
-   0xb7, 0xe6, 0x2b, 0x34, 0xec, 0x93, 0x1e, 0x23,
-   };
+   static const uuid_le intel_dsm_uuid =
+   UUID_LE(0xac340cb7, 0xe901, 0x45bf,
+   0xb7, 0xe6, 0x2b, 0x34, 0xec, 0x93, 0x1e, 0x23);
union acpi_object *obj;

-   obj = acpi_evaluate_dsm(ACPI_HANDLE(>dev), intel_dsm_uuid, 3, 1,
+   obj = acpi_evaluate_dsm(ACPI_HANDLE(>dev), _dsm_uuid, 3, 1,
NULL);
ACPI_FREE(obj);
  }


For the xhci part above:

Acked-by: Mathias Nyman 


___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau


Re: [Nouveau] [PATCH v1] ACPI: Switch to use generic UUID API

2017-05-15 Thread Zhang Rui
On Thu, 2017-05-04 at 12:21 +0300, Andy Shevchenko wrote:
> acpi_evaluate_dsm() and friends take a pointer to a raw buffer of 16
> bytes. Instead we convert them to use uuid_le type. At the same time
> we
> convert current users.
> 
> acpi_str_to_uuid() becomes useless after the conversion and it's safe
> to
> get rid of it.
> 
> The conversion fixes a potential bug in int340x_thermal as well since
> we have to use memcmp() on binary data.
> 
> Cc: Rafael J. Wysocki 
> Cc: Mika Westerberg 
> Cc: Borislav Petkov 
> Cc: Dan Williams 
> Cc: Amir Goldstein 
> Cc: Jarkko Sakkinen 
> Cc: Jani Nikula 
> Cc: Ben Skeggs 
> Cc: Benjamin Tissoires 
> Cc: Joerg Roedel 
> Cc: Adrian Hunter 
> Cc: Yisen Zhuang 
> Cc: Bjorn Helgaas 
> Cc: Zhang Rui 
> Cc: Felipe Balbi 
> Cc: Mathias Nyman 
> Cc: Heikki Krogerus 
> Cc: Liam Girdwood 
> Cc: Mark Brown 
> Signed-off-by: Andy Shevchenko 
> ---
> 
> diff --git a/drivers/thermal/int340x_thermal/int3400_thermal.c
> b/drivers/thermal/int340x_thermal/int3400_thermal.c
> index 9413c4abf0b9..c0eb3bb19b23 100644
> --- a/drivers/thermal/int340x_thermal/int3400_thermal.c
> +++ b/drivers/thermal/int340x_thermal/int3400_thermal.c
> @@ -23,7 +23,7 @@ enum int3400_thermal_uuid {
>   INT3400_THERMAL_MAXIMUM_UUID,
>  };
>  
> -static u8 *int3400_thermal_uuids[INT3400_THERMAL_MAXIMUM_UUID] = {
> +static const char
> *int3400_thermal_uuids[INT3400_THERMAL_MAXIMUM_UUID] = {
>   "42A441D6-AE6A-462b-A84B-4A8CE79027D3",
>   "3A95C389-E4B8-4629-A526-C52C88626BAE",
>   "97C68AE7-15FA-499c-B8C9-5DA81D606E0A",
> @@ -141,10 +141,10 @@ static int int3400_thermal_get_uuids(struct
> int3400_thermal_priv *priv)
>   }
>  
>   for (j = 0; j < INT3400_THERMAL_MAXIMUM_UUID; j++) {
> - u8 uuid[16];
> + uuid_le u;
>  
> - acpi_str_to_uuid(int3400_thermal_uuids[j],
> uuid);
> - if (!strncmp(uuid, objb->buffer.pointer,
> 16)) {
> + uuid_le_to_bin(int3400_thermal_uuids[j],
> );
> + if (!uuid_le_cmp(*(uuid_le *)objb-
> >buffer.pointer), u) {
>   priv->uuid_bitmap |= (1 << j);
>   break;
>   }
thanks for the fix.

Acked-by: Zhang Rui 

-rui
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau


Re: [Nouveau] [PATCH v1] ACPI: Switch to use generic UUID API

2017-05-15 Thread Felipe Balbi

Hi,

Andy Shevchenko  writes:
> acpi_evaluate_dsm() and friends take a pointer to a raw buffer of 16
> bytes. Instead we convert them to use uuid_le type. At the same time we
> convert current users.
>
> acpi_str_to_uuid() becomes useless after the conversion and it's safe to
> get rid of it.
>
> The conversion fixes a potential bug in int340x_thermal as well since
> we have to use memcmp() on binary data.
>
> Cc: Rafael J. Wysocki 
> Cc: Mika Westerberg 
> Cc: Borislav Petkov 
> Cc: Dan Williams 
> Cc: Amir Goldstein 
> Cc: Jarkko Sakkinen 
> Cc: Jani Nikula 
> Cc: Ben Skeggs 
> Cc: Benjamin Tissoires 
> Cc: Joerg Roedel 
> Cc: Adrian Hunter 
> Cc: Yisen Zhuang 
> Cc: Bjorn Helgaas 
> Cc: Zhang Rui 
> Cc: Felipe Balbi 
> Cc: Mathias Nyman 
> Cc: Heikki Krogerus 
> Cc: Liam Girdwood 
> Cc: Mark Brown 
> Signed-off-by: Andy Shevchenko 
> diff --git a/drivers/usb/dwc3/dwc3-pci.c b/drivers/usb/dwc3/dwc3-pci.c
> index a15ec71d0423..6b5284ec76df 100644
> --- a/drivers/usb/dwc3/dwc3-pci.c
> +++ b/drivers/usb/dwc3/dwc3-pci.c
> @@ -56,7 +56,7 @@ struct dwc3_pci {
>   struct platform_device *dwc3;
>   struct pci_dev *pci;
>  
> - u8 uuid[16];
> + uuid_le uuid;
>  
>   unsigned int has_dsm_for_pm:1;
>  };
> @@ -118,7 +118,7 @@ static int dwc3_pci_quirks(struct dwc3_pci *dwc)
>  
>   if (pdev->device == PCI_DEVICE_ID_INTEL_BXT ||
>   pdev->device == PCI_DEVICE_ID_INTEL_BXT_M) {
> - acpi_str_to_uuid(PCI_INTEL_BXT_DSM_UUID, dwc->uuid);
> + uuid_le_to_bin(PCI_INTEL_BXT_DSM_UUID, >uuid);
>   dwc->has_dsm_for_pm = true;
>   }
>  
> @@ -288,7 +288,7 @@ static int dwc3_pci_dsm(struct dwc3_pci *dwc, int param)
>   tmp.type = ACPI_TYPE_INTEGER;
>   tmp.integer.value = param;
>  
> - obj = acpi_evaluate_dsm(ACPI_HANDLE(>pci->dev), dwc->uuid,
> + obj = acpi_evaluate_dsm(ACPI_HANDLE(>pci->dev), >uuid,
>   1, PCI_INTEL_BXT_FUNC_PMU_PWR, );
>   if (!obj) {
>   dev_err(>pci->dev, "failed to evaluate _DSM\n");

Acked-by: Felipe Balbi 

-- 
balbi
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau


Re: [Nouveau] [PATCH v1] ACPI: Switch to use generic UUID API

2017-05-06 Thread Christoph Hellwig
On Fri, May 05, 2017 at 10:06:06AM +0300, Amir Goldstein wrote:
> I much rather that you sort out uuid helpers in a way that will
> satisfy the filesystem
> needs (just provide the helpers don't need to convert filesystems code).

Yeah.

> IMO, you should acknowledge that the common use case for filesystems is
> to handle an opaque char[16] which most likely holds a uuid_be and you
> should provide 'neutral' helpers to satisfy this use case.
> 
> The simplest would be to typedef uuid_t to struct uuid_be and to name 
> 'neutral'
> helpers' uuid_cmp/uuid_copy(uuid_t *, uuid_t *), similar to my proposal.

It's not jut neutral, it's the right thing to to.  The Apollo / DCE
uuids (later also specified in RFC 4122) are big endian, so we should
use the name there.

> Christoph also suggested a similar treatment to typedef guid_t to
> struct uuid_le.

Exactly.  The whole idea of "little endian UUIDs" comes from the
Wintel world, and if you look at the relevant specs they are almost
exclusively referred to as GUIDs.

The magic XFS and AFS types for specific interpretations of one of
the RFC4122 formats don't really help, but I'll just send a patch
to kill them off for XFS ASAP to at least get that out, and we
probably should revert at least

"afs: Move UUID struct to linux/uuid.h"

That moved the AFS mess to common code as a start, and then
also clean up the way we generate random UUIDs, where we currently
have le helper, a be helper and then also generate_random_uuid
just to confuse the heck out of people.  With no description of
either of them.
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau


Re: [Nouveau] [PATCH v1] ACPI: Switch to use generic UUID API

2017-05-06 Thread Amir Goldstein
On Fri, May 5, 2017 at 12:24 PM, Andy Shevchenko
 wrote:
> On Fri, 2017-05-05 at 10:06 +0300, Amir Goldstein wrote:
[...]
>> I think with this semantic change, our proposals can reach common
>> grounds
>> and satisfy a wider group of users (i.e. filesystem developers).
>>
>> Christoph also suggested a similar treatment to typedef guid_t to
>> struct uuid_le.
>> I don't know the use cases enough to comment on that.
>
> We may go this way. But I wouldn't prevent current users of uuid_le to
> continue using it without conversion (it may be done case by case after
> we settle an API)
>
> So, summarize what Christoph said it will look like
>
> typedef uuid_be uuid_t;
> typedef uuid_le guid_t
>
> uuid_cmp() / uuid_copy() / uuid_to_bin() / etc
> guid_cmp() / guid_copy() / guid_to_bin() / etc
>
> Correct? Christoph?
>

That looks right to me.

To complete the picture for folks not cc'ed on my patches,
xfs use case suggests there is also justification for the additional helpers:

uuid_is_null() / uuid_equal()
guid_is_null() / guid_equal()

Cheers,
Amir.
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau


Re: [Nouveau] [PATCH v1] ACPI: Switch to use generic UUID API

2017-05-06 Thread Christoph Hellwig
On Fri, May 05, 2017 at 12:50:31PM +0300, Amir Goldstein wrote:
> To complete the picture for folks not cc'ed on my patches,
> xfs use case suggests there is also justification for the additional helpers:
> 
> uuid_is_null() / uuid_equal()
> guid_is_null() / guid_equal()

The is_null is useful and I bet we'll find other instances.  I'm
not sure _equals really adds much value over the existing _cmp
helpers, but on the other hand they are so trivial that we might as
well add them.

The other thing XFS has is uuid_copy.
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau


Re: [Nouveau] [PATCH v1] ACPI: Switch to use generic UUID API

2017-05-06 Thread Benjamin Tissoires
On May 04 2017 or thereabouts, Andy Shevchenko wrote:
> acpi_evaluate_dsm() and friends take a pointer to a raw buffer of 16
> bytes. Instead we convert them to use uuid_le type. At the same time we
> convert current users.
> 
> acpi_str_to_uuid() becomes useless after the conversion and it's safe to
> get rid of it.
> 
> The conversion fixes a potential bug in int340x_thermal as well since
> we have to use memcmp() on binary data.
> 
> Cc: Rafael J. Wysocki 
> Cc: Mika Westerberg 
> Cc: Borislav Petkov 
> Cc: Dan Williams 
> Cc: Amir Goldstein 
> Cc: Jarkko Sakkinen 
> Cc: Jani Nikula 
> Cc: Ben Skeggs 
> Cc: Benjamin Tissoires 
> Cc: Joerg Roedel 
> Cc: Adrian Hunter 
> Cc: Yisen Zhuang 
> Cc: Bjorn Helgaas 
> Cc: Zhang Rui 
> Cc: Felipe Balbi 
> Cc: Mathias Nyman 
> Cc: Heikki Krogerus 
> Cc: Liam Girdwood 
> Cc: Mark Brown 
> Signed-off-by: Andy Shevchenko 
> ---

For i2c-hid:
Acked-by: Benjamin Tissoires 
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau


Re: [Nouveau] [PATCH v1] ACPI: Switch to use generic UUID API

2017-05-06 Thread Amir Goldstein
On Fri, May 5, 2017 at 12:57 PM, Christoph Hellwig  wrote:
> On Fri, May 05, 2017 at 12:50:31PM +0300, Amir Goldstein wrote:
>> To complete the picture for folks not cc'ed on my patches,
>> xfs use case suggests there is also justification for the additional helpers:
>>
>> uuid_is_null() / uuid_equal()
>> guid_is_null() / guid_equal()
>
> The is_null is useful and I bet we'll find other instances. I'm
> not sure _equals really adds much value over the existing _cmp
> helpers, but on the other hand they are so trivial that we might as
> well add them.

Exactly. The fact that not only xfs used the same helper name
(drivers/md/md.c) suggests that it useful.

>
> The other thing XFS has is uuid_copy.

Andy already listed uuid_copy.
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau


Re: [Nouveau] [PATCH v1] ACPI: Switch to use generic UUID API

2017-05-06 Thread Heikki Krogerus
On Thu, May 04, 2017 at 12:21:51PM +0300, Andy Shevchenko wrote:
> acpi_evaluate_dsm() and friends take a pointer to a raw buffer of 16
> bytes. Instead we convert them to use uuid_le type. At the same time we
> convert current users.
> 
> acpi_str_to_uuid() becomes useless after the conversion and it's safe to
> get rid of it.
> 
> The conversion fixes a potential bug in int340x_thermal as well since
> we have to use memcmp() on binary data.
> 
> Cc: Rafael J. Wysocki 
> Cc: Mika Westerberg 
> Cc: Borislav Petkov 
> Cc: Dan Williams 
> Cc: Amir Goldstein 
> Cc: Jarkko Sakkinen 
> Cc: Jani Nikula 
> Cc: Ben Skeggs 
> Cc: Benjamin Tissoires 
> Cc: Joerg Roedel 
> Cc: Adrian Hunter 
> Cc: Yisen Zhuang 
> Cc: Bjorn Helgaas 
> Cc: Zhang Rui 
> Cc: Felipe Balbi 
> Cc: Mathias Nyman 
> Cc: Heikki Krogerus 
> Cc: Liam Girdwood 
> Cc: Mark Brown 
> Signed-off-by: Andy Shevchenko 

OK by me, FWIW:

Reviewed-by: Heikki Krogerus 


Thanks,

-- 
heikki
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau


Re: [Nouveau] [PATCH v1] ACPI: Switch to use generic UUID API

2017-05-06 Thread Jani Nikula
On Thu, 04 May 2017, Andy Shevchenko  wrote:
> diff --git a/drivers/gpu/drm/i915/intel_acpi.c 
> b/drivers/gpu/drm/i915/intel_acpi.c
> index eb638a1e69d2..72bfe6ceadf8 100644
> --- a/drivers/gpu/drm/i915/intel_acpi.c
> +++ b/drivers/gpu/drm/i915/intel_acpi.c
> @@ -15,13 +15,9 @@ static struct intel_dsm_priv {
>   acpi_handle dhandle;
>  } intel_dsm_priv;
>  
> -static const u8 intel_dsm_guid[] = {
> - 0xd3, 0x73, 0xd8, 0x7e,
> - 0xd0, 0xc2,
> - 0x4f, 0x4e,
> - 0xa8, 0x54,
> - 0x0f, 0x13, 0x17, 0xb0, 0x1c, 0x2c
> -};
> +static const uuid_le intel_dsm_guid =
> + UUID_LE(0x7ed873d3, 0xc2d0, 0x4e4f,
> + 0xa8, 0x54, 0x0f, 0x13, 0x17, 0xb0, 0x1c, 0x2c);
>  
>  static char *intel_dsm_port_name(u8 id)
>  {
> @@ -80,7 +76,7 @@ static void intel_dsm_platform_mux_info(void)
>   int i;
>   union acpi_object *pkg, *connector_count;
>  
> - pkg = acpi_evaluate_dsm_typed(intel_dsm_priv.dhandle, intel_dsm_guid,
> + pkg = acpi_evaluate_dsm_typed(intel_dsm_priv.dhandle, _dsm_guid,
>   INTEL_DSM_REVISION_ID, INTEL_DSM_FN_PLATFORM_MUX_INFO,
>   NULL, ACPI_TYPE_PACKAGE);
>   if (!pkg) {
> @@ -118,7 +114,7 @@ static bool intel_dsm_pci_probe(struct pci_dev *pdev)
>   if (!dhandle)
>   return false;
>  
> - if (!acpi_check_dsm(dhandle, intel_dsm_guid, INTEL_DSM_REVISION_ID,
> + if (!acpi_check_dsm(dhandle, _dsm_guid, INTEL_DSM_REVISION_ID,
>   1 << INTEL_DSM_FN_PLATFORM_MUX_INFO)) {
>   DRM_DEBUG_KMS("no _DSM method for intel device\n");
>   return false;

The drm/i915 hunk above is

Reviewed-by: Jani Nikula 

and acked for merging via whichever tree is suitable.


BR,
Jani.

-- 
Jani Nikula, Intel Open Source Technology Center
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau


Re: [Nouveau] [PATCH v1] ACPI: Switch to use generic UUID API

2017-05-06 Thread Andy Shevchenko
On Fri, 2017-05-05 at 10:06 +0300, Amir Goldstein wrote:
> On Fri, May 5, 2017 at 9:20 AM, Dan Williams  > wrote:
> > On Thu, May 4, 2017 at 2:21 AM, Andy Shevchenko
> >  wrote:


> > > for (i = 0; i < NFIT_UUID_MAX; i++)
> > > -   if (memcmp(to_nfit_uuid(i), spa->range_guid, 16)
> > > == 0)
> > > +   if (!uuid_le_cmp_pp(to_nfit_uuid(i), (uuid_le
> > > *)spa->range_guid))
> > 
> > What is _cmp_pp? Why not _compare?

Dan, it's a typo. In this patch it should be just ..._cmp(), which is
already a part of API.

> > 
> 
> I second that.
> 
> Andy,

Amir, just to be clear. This patch can be applied without any addons to
an existing API. Above is just a typo due to rebase in my tree. I will
replace it to just uuid_le_cmp().

> I much rather that you sort out uuid helpers in a way that will
> satisfy the filesystem
> needs (just provide the helpers don't need to convert filesystems
> code).

> The only reason I took a swing at hoisting the xfs uuid helpers is
> because it didn't
> seem like your proposal was going to be posted soon or wasn't going to
> satisfy
> the filesystems use case.

> 
> My opinion now, is that your suggestion is probably much closer to the
> real deal
> than mine.
> 
> IMO, you should acknowledge that the common use case for filesystems
> is
> to handle an opaque char[16] which most likely holds a uuid_be and you
> should provide 'neutral' helpers to satisfy this use case.
> 
> The simplest would be to typedef uuid_t to struct uuid_be and to name
> 'neutral'
> helpers' uuid_cmp/uuid_copy(uuid_t *, uuid_t *), similar to my
> proposal.

> I think with this semantic change, our proposals can reach common
> grounds
> and satisfy a wider group of users (i.e. filesystem developers).
> 
> Christoph also suggested a similar treatment to typedef guid_t to
> struct uuid_le.
> I don't know the use cases enough to comment on that.

We may go this way. But I wouldn't prevent current users of uuid_le to
continue using it without conversion (it may be done case by case after
we settle an API)

So, summarize what Christoph said it will look like

typedef uuid_be uuid_t;
typedef uuid_le guid_t

uuid_cmp() / uuid_copy() / uuid_to_bin() / etc
guid_cmp() / guid_copy() / guid_to_bin() / etc

Correct? Christoph?

-- 
Andy Shevchenko 
Intel Finland Oy
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau


Re: [Nouveau] [PATCH v1] ACPI: Switch to use generic UUID API

2017-05-06 Thread Joerg Roedel
On Thu, May 04, 2017 at 12:21:51PM +0300, Andy Shevchenko wrote:
> diff --git a/drivers/iommu/dmar.c b/drivers/iommu/dmar.c
> index cbf7763d8091..420d51b286ad 100644
> --- a/drivers/iommu/dmar.c
> +++ b/drivers/iommu/dmar.c
> @@ -1808,10 +1808,9 @@ IOMMU_INIT_POST(detect_intel_iommu);
>   * for Directed-IO Architecture Specifiction, Rev 2.2, Section 8.8
>   * "Remapping Hardware Unit Hot Plug".
>   */
> -static u8 dmar_hp_uuid[] = {
> - /*  */0xA6, 0xA3, 0xC1, 0xD8, 0x9B, 0xBE, 0x9B, 0x4C,
> - /* 0008 */0x91, 0xBF, 0xC3, 0xCB, 0x81, 0xFC, 0x5D, 0xAF
> -};
> +static uuid_le dmar_hp_uuid =
> + UUID_LE(0xD8C1A3A6, 0xBE9B, 0x4C9B,
> + 0x91, 0xBF, 0xC3, 0xCB, 0x81, 0xFC, 0x5D, 0xAF);
>  
>  /*
>   * Currently there's only one revision and BIOS will not check the revision 
> id,
> @@ -1824,7 +1823,7 @@ static u8 dmar_hp_uuid[] = {
>  
>  static inline bool dmar_detect_dsm(acpi_handle handle, int func)
>  {
> - return acpi_check_dsm(handle, dmar_hp_uuid, DMAR_DSM_REV_ID, 1 << func);
> + return acpi_check_dsm(handle, _hp_uuid, DMAR_DSM_REV_ID, 1 << 
> func);
>  }
>  
>  static int dmar_walk_dsm_resource(acpi_handle handle, int func,
> @@ -1843,7 +1842,7 @@ static int dmar_walk_dsm_resource(acpi_handle handle, 
> int func,
>   if (!dmar_detect_dsm(handle, func))
>   return 0;
>  
> - obj = acpi_evaluate_dsm_typed(handle, dmar_hp_uuid, DMAR_DSM_REV_ID,
> + obj = acpi_evaluate_dsm_typed(handle, _hp_uuid, DMAR_DSM_REV_ID,
> func, NULL, ACPI_TYPE_BUFFER);
>   if (!obj)
>   return -ENODEV;

DMAR part is

Acked-by: Joerg Roedel 
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau


Re: [Nouveau] [PATCH v1] ACPI: Switch to use generic UUID API

2017-05-06 Thread Bjorn Helgaas
On Thu, May 4, 2017 at 4:21 AM, Andy Shevchenko
 wrote:
> acpi_evaluate_dsm() and friends take a pointer to a raw buffer of 16
> bytes. Instead we convert them to use uuid_le type. At the same time we
> convert current users.
>
> acpi_str_to_uuid() becomes useless after the conversion and it's safe to
> get rid of it.
>
> The conversion fixes a potential bug in int340x_thermal as well since
> we have to use memcmp() on binary data.
>
> Cc: Rafael J. Wysocki 
> Cc: Mika Westerberg 
> Cc: Borislav Petkov 
> Cc: Dan Williams 
> Cc: Amir Goldstein 
> Cc: Jarkko Sakkinen 
> Cc: Jani Nikula 
> Cc: Ben Skeggs 
> Cc: Benjamin Tissoires 
> Cc: Joerg Roedel 
> Cc: Adrian Hunter 
> Cc: Yisen Zhuang 
> Cc: Bjorn Helgaas 
> Cc: Zhang Rui 
> Cc: Felipe Balbi 
> Cc: Mathias Nyman 
> Cc: Heikki Krogerus 
> Cc: Liam Girdwood 
> Cc: Mark Brown 
> Signed-off-by: Andy Shevchenko 

For the drivers/pci parts:

Acked-by: Bjorn Helgaas 

> ---
>  drivers/acpi/acpi_extlog.c | 10 +++---
>  drivers/acpi/bus.c | 29 ++--
>  drivers/acpi/nfit/core.c   | 40 
> +++---
>  drivers/acpi/nfit/nfit.h   |  3 +-
>  drivers/acpi/utils.c   |  4 +--
>  drivers/char/tpm/tpm_crb.c |  9 +++--
>  drivers/char/tpm/tpm_ppi.c | 20 +--
>  drivers/gpu/drm/i915/intel_acpi.c  | 14 +++-
>  drivers/gpu/drm/nouveau/nouveau_acpi.c | 20 +--
>  drivers/gpu/drm/nouveau/nvkm/subdev/mxm/base.c |  9 +++--
>  drivers/hid/i2c-hid/i2c-hid.c  |  9 +++--
>  drivers/iommu/dmar.c   | 11 +++---
>  drivers/mmc/host/sdhci-pci-core.c  |  9 +++--
>  drivers/net/ethernet/hisilicon/hns/hns_dsaf_misc.c | 15 
>  drivers/pci/pci-acpi.c | 11 +++---
>  drivers/pci/pci-label.c|  4 +--
>  drivers/thermal/int340x_thermal/int3400_thermal.c  |  8 ++---
>  drivers/usb/dwc3/dwc3-pci.c|  6 ++--
>  drivers/usb/host/xhci-pci.c|  9 +++--
>  drivers/usb/misc/ucsi.c|  2 +-
>  drivers/usb/typec/typec_wcove.c|  4 +--
>  include/acpi/acpi_bus.h|  9 ++---
>  include/linux/acpi.h   |  4 +--
>  include/linux/pci-acpi.h   |  2 +-
>  sound/soc/intel/skylake/skl-nhlt.c |  7 ++--
>  tools/testing/nvdimm/test/iomap.c  |  2 +-
>  tools/testing/nvdimm/test/nfit.c   |  2 +-
>  27 files changed, 116 insertions(+), 156 deletions(-)
>
> diff --git a/drivers/acpi/acpi_extlog.c b/drivers/acpi/acpi_extlog.c
> index 502ea4dc2080..69d6140b6afa 100644
> --- a/drivers/acpi/acpi_extlog.c
> +++ b/drivers/acpi/acpi_extlog.c
> @@ -182,17 +182,17 @@ static int extlog_print(struct notifier_block *nb, 
> unsigned long val,
>
>  static bool __init extlog_get_l1addr(void)
>  {
> -   u8 uuid[16];
> +   uuid_le uuid;
> acpi_handle handle;
> union acpi_object *obj;
>
> -   acpi_str_to_uuid(extlog_dsm_uuid, uuid);
> -
> +   if (uuid_le_to_bin(extlog_dsm_uuid, ))
> +   return false;
> if (ACPI_FAILURE(acpi_get_handle(NULL, "\\_SB", )))
> return false;
> -   if (!acpi_check_dsm(handle, uuid, EXTLOG_DSM_REV, 1 << 
> EXTLOG_FN_ADDR))
> +   if (!acpi_check_dsm(handle, , EXTLOG_DSM_REV, 1 << 
> EXTLOG_FN_ADDR))
> return false;
> -   obj = acpi_evaluate_dsm_typed(handle, uuid, EXTLOG_DSM_REV,
> +   obj = acpi_evaluate_dsm_typed(handle, , EXTLOG_DSM_REV,
>   EXTLOG_FN_ADDR, NULL, 
> ACPI_TYPE_INTEGER);
> if (!obj) {
> return false;
> diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
> index 784bda663d16..e8130a4873e9 100644
> --- a/drivers/acpi/bus.c
> +++ b/drivers/acpi/bus.c
> @@ -196,42 +196,19 @@ static void acpi_print_osc_error(acpi_handle handle,
> pr_debug("\n");
>  }
>
> -acpi_status acpi_str_to_uuid(char *str, u8 *uuid)
> -{
> -   int i;
> -   static int opc_map_to_uuid[16] = {6, 4, 2, 0, 11, 9, 16, 14, 19, 21,
> -   24, 26, 28, 30, 32, 34};
> -
> -   if (strlen(str) != 36)
> -   return 

Re: [Nouveau] [PATCH v1] ACPI: Switch to use generic UUID API

2017-05-06 Thread Dan Williams
On Thu, May 4, 2017 at 2:21 AM, Andy Shevchenko
 wrote:
> acpi_evaluate_dsm() and friends take a pointer to a raw buffer of 16
> bytes. Instead we convert them to use uuid_le type. At the same time we
> convert current users.
>
> acpi_str_to_uuid() becomes useless after the conversion and it's safe to
> get rid of it.
>
> The conversion fixes a potential bug in int340x_thermal as well since
> we have to use memcmp() on binary data.
>
> Cc: Rafael J. Wysocki 
> Cc: Mika Westerberg 
> Cc: Borislav Petkov 
> Cc: Dan Williams 
> Cc: Amir Goldstein 
> Cc: Jarkko Sakkinen 
> Cc: Jani Nikula 
> Cc: Ben Skeggs 
> Cc: Benjamin Tissoires 
> Cc: Joerg Roedel 
> Cc: Adrian Hunter 
> Cc: Yisen Zhuang 
> Cc: Bjorn Helgaas 
> Cc: Zhang Rui 
> Cc: Felipe Balbi 
> Cc: Mathias Nyman 
> Cc: Heikki Krogerus 
> Cc: Liam Girdwood 
> Cc: Mark Brown 
> Signed-off-by: Andy Shevchenko 
[..]
> diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
> index 0f7982a1caaf..bd3e45ede056 100644
> --- a/drivers/acpi/nfit/core.c
> +++ b/drivers/acpi/nfit/core.c
> @@ -74,11 +74,11 @@ struct nfit_table_prev {
> struct list_head flushes;
>  };
>
> -static u8 nfit_uuid[NFIT_UUID_MAX][16];
> +static uuid_le nfit_uuid[NFIT_UUID_MAX];
>
> -const u8 *to_nfit_uuid(enum nfit_uuids id)
> +const uuid_le *to_nfit_uuid(enum nfit_uuids id)
>  {
> -   return nfit_uuid[id];
> +   return _uuid[id];
>  }
>  EXPORT_SYMBOL(to_nfit_uuid);
>
> @@ -207,7 +207,7 @@ int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc, 
> struct nvdimm *nvdimm,
> u32 offset, fw_status = 0;
> acpi_handle handle;
> unsigned int func;
> -   const u8 *uuid;
> +   const uuid_le *uuid;
> int rc, i;
>
> func = cmd;
> @@ -394,7 +394,7 @@ int nfit_spa_type(struct acpi_nfit_system_address *spa)
> int i;
>
> for (i = 0; i < NFIT_UUID_MAX; i++)
> -   if (memcmp(to_nfit_uuid(i), spa->range_guid, 16) == 0)
> +   if (!uuid_le_cmp_pp(to_nfit_uuid(i), (uuid_le 
> *)spa->range_guid))

What is _cmp_pp? Why not _compare?

Other than that, looks ok to me.
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau


Re: [Nouveau] [PATCH v1] ACPI: Switch to use generic UUID API

2017-05-06 Thread Amir Goldstein
On Fri, May 5, 2017 at 9:20 AM, Dan Williams  wrote:
> On Thu, May 4, 2017 at 2:21 AM, Andy Shevchenko
>  wrote:
>> acpi_evaluate_dsm() and friends take a pointer to a raw buffer of 16
>> bytes. Instead we convert them to use uuid_le type. At the same time we
>> convert current users.
>>
>> acpi_str_to_uuid() becomes useless after the conversion and it's safe to
>> get rid of it.
>>
>> The conversion fixes a potential bug in int340x_thermal as well since
>> we have to use memcmp() on binary data.
>>
>> Cc: Rafael J. Wysocki 
>> Cc: Mika Westerberg 
>> Cc: Borislav Petkov 
>> Cc: Dan Williams 
>> Cc: Amir Goldstein 
>> Cc: Jarkko Sakkinen 
>> Cc: Jani Nikula 
>> Cc: Ben Skeggs 
>> Cc: Benjamin Tissoires 
>> Cc: Joerg Roedel 
>> Cc: Adrian Hunter 
>> Cc: Yisen Zhuang 
>> Cc: Bjorn Helgaas 
>> Cc: Zhang Rui 
>> Cc: Felipe Balbi 
>> Cc: Mathias Nyman 
>> Cc: Heikki Krogerus 
>> Cc: Liam Girdwood 
>> Cc: Mark Brown 
>> Signed-off-by: Andy Shevchenko 
> [..]
>> diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
>> index 0f7982a1caaf..bd3e45ede056 100644
>> --- a/drivers/acpi/nfit/core.c
>> +++ b/drivers/acpi/nfit/core.c
>> @@ -74,11 +74,11 @@ struct nfit_table_prev {
>> struct list_head flushes;
>>  };
>>
>> -static u8 nfit_uuid[NFIT_UUID_MAX][16];
>> +static uuid_le nfit_uuid[NFIT_UUID_MAX];
>>
>> -const u8 *to_nfit_uuid(enum nfit_uuids id)
>> +const uuid_le *to_nfit_uuid(enum nfit_uuids id)
>>  {
>> -   return nfit_uuid[id];
>> +   return _uuid[id];
>>  }
>>  EXPORT_SYMBOL(to_nfit_uuid);
>>
>> @@ -207,7 +207,7 @@ int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc, 
>> struct nvdimm *nvdimm,
>> u32 offset, fw_status = 0;
>> acpi_handle handle;
>> unsigned int func;
>> -   const u8 *uuid;
>> +   const uuid_le *uuid;
>> int rc, i;
>>
>> func = cmd;
>> @@ -394,7 +394,7 @@ int nfit_spa_type(struct acpi_nfit_system_address *spa)
>> int i;
>>
>> for (i = 0; i < NFIT_UUID_MAX; i++)
>> -   if (memcmp(to_nfit_uuid(i), spa->range_guid, 16) == 0)
>> +   if (!uuid_le_cmp_pp(to_nfit_uuid(i), (uuid_le 
>> *)spa->range_guid))
>
> What is _cmp_pp? Why not _compare?
>

I second that.

Andy,

I much rather that you sort out uuid helpers in a way that will
satisfy the filesystem
needs (just provide the helpers don't need to convert filesystems code).

The only reason I took a swing at hoisting the xfs uuid helpers is
because it didn't
seem like your proposal was going to be posted soon or wasn't going to satisfy
the filesystems use case.

My opinion now, is that your suggestion is probably much closer to the real deal
than mine.

IMO, you should acknowledge that the common use case for filesystems is
to handle an opaque char[16] which most likely holds a uuid_be and you
should provide 'neutral' helpers to satisfy this use case.

The simplest would be to typedef uuid_t to struct uuid_be and to name 'neutral'
helpers' uuid_cmp/uuid_copy(uuid_t *, uuid_t *), similar to my proposal.

I think with this semantic change, our proposals can reach common grounds
and satisfy a wider group of users (i.e. filesystem developers).

Christoph also suggested a similar treatment to typedef guid_t to
struct uuid_le.
I don't know the use cases enough to comment on that.

Cheers,
Amir.
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau


[Nouveau] [PATCH v1] ACPI: Switch to use generic UUID API

2017-05-06 Thread Andy Shevchenko
acpi_evaluate_dsm() and friends take a pointer to a raw buffer of 16
bytes. Instead we convert them to use uuid_le type. At the same time we
convert current users.

acpi_str_to_uuid() becomes useless after the conversion and it's safe to
get rid of it.

The conversion fixes a potential bug in int340x_thermal as well since
we have to use memcmp() on binary data.

Cc: Rafael J. Wysocki 
Cc: Mika Westerberg 
Cc: Borislav Petkov 
Cc: Dan Williams 
Cc: Amir Goldstein 
Cc: Jarkko Sakkinen 
Cc: Jani Nikula 
Cc: Ben Skeggs 
Cc: Benjamin Tissoires 
Cc: Joerg Roedel 
Cc: Adrian Hunter 
Cc: Yisen Zhuang 
Cc: Bjorn Helgaas 
Cc: Zhang Rui 
Cc: Felipe Balbi 
Cc: Mathias Nyman 
Cc: Heikki Krogerus 
Cc: Liam Girdwood 
Cc: Mark Brown 
Signed-off-by: Andy Shevchenko 
---
 drivers/acpi/acpi_extlog.c | 10 +++---
 drivers/acpi/bus.c | 29 ++--
 drivers/acpi/nfit/core.c   | 40 +++---
 drivers/acpi/nfit/nfit.h   |  3 +-
 drivers/acpi/utils.c   |  4 +--
 drivers/char/tpm/tpm_crb.c |  9 +++--
 drivers/char/tpm/tpm_ppi.c | 20 +--
 drivers/gpu/drm/i915/intel_acpi.c  | 14 +++-
 drivers/gpu/drm/nouveau/nouveau_acpi.c | 20 +--
 drivers/gpu/drm/nouveau/nvkm/subdev/mxm/base.c |  9 +++--
 drivers/hid/i2c-hid/i2c-hid.c  |  9 +++--
 drivers/iommu/dmar.c   | 11 +++---
 drivers/mmc/host/sdhci-pci-core.c  |  9 +++--
 drivers/net/ethernet/hisilicon/hns/hns_dsaf_misc.c | 15 
 drivers/pci/pci-acpi.c | 11 +++---
 drivers/pci/pci-label.c|  4 +--
 drivers/thermal/int340x_thermal/int3400_thermal.c  |  8 ++---
 drivers/usb/dwc3/dwc3-pci.c|  6 ++--
 drivers/usb/host/xhci-pci.c|  9 +++--
 drivers/usb/misc/ucsi.c|  2 +-
 drivers/usb/typec/typec_wcove.c|  4 +--
 include/acpi/acpi_bus.h|  9 ++---
 include/linux/acpi.h   |  4 +--
 include/linux/pci-acpi.h   |  2 +-
 sound/soc/intel/skylake/skl-nhlt.c |  7 ++--
 tools/testing/nvdimm/test/iomap.c  |  2 +-
 tools/testing/nvdimm/test/nfit.c   |  2 +-
 27 files changed, 116 insertions(+), 156 deletions(-)

diff --git a/drivers/acpi/acpi_extlog.c b/drivers/acpi/acpi_extlog.c
index 502ea4dc2080..69d6140b6afa 100644
--- a/drivers/acpi/acpi_extlog.c
+++ b/drivers/acpi/acpi_extlog.c
@@ -182,17 +182,17 @@ static int extlog_print(struct notifier_block *nb, 
unsigned long val,
 
 static bool __init extlog_get_l1addr(void)
 {
-   u8 uuid[16];
+   uuid_le uuid;
acpi_handle handle;
union acpi_object *obj;
 
-   acpi_str_to_uuid(extlog_dsm_uuid, uuid);
-
+   if (uuid_le_to_bin(extlog_dsm_uuid, ))
+   return false;
if (ACPI_FAILURE(acpi_get_handle(NULL, "\\_SB", )))
return false;
-   if (!acpi_check_dsm(handle, uuid, EXTLOG_DSM_REV, 1 << EXTLOG_FN_ADDR))
+   if (!acpi_check_dsm(handle, , EXTLOG_DSM_REV, 1 << EXTLOG_FN_ADDR))
return false;
-   obj = acpi_evaluate_dsm_typed(handle, uuid, EXTLOG_DSM_REV,
+   obj = acpi_evaluate_dsm_typed(handle, , EXTLOG_DSM_REV,
  EXTLOG_FN_ADDR, NULL, ACPI_TYPE_INTEGER);
if (!obj) {
return false;
diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index 784bda663d16..e8130a4873e9 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -196,42 +196,19 @@ static void acpi_print_osc_error(acpi_handle handle,
pr_debug("\n");
 }
 
-acpi_status acpi_str_to_uuid(char *str, u8 *uuid)
-{
-   int i;
-   static int opc_map_to_uuid[16] = {6, 4, 2, 0, 11, 9, 16, 14, 19, 21,
-   24, 26, 28, 30, 32, 34};
-
-   if (strlen(str) != 36)
-   return AE_BAD_PARAMETER;
-   for (i = 0; i < 36; i++) {
-   if (i == 8 || i == 13 || i == 18 || i == 23) {
-   if (str[i] != '-')
-   return AE_BAD_PARAMETER;
-   } else if (!isxdigit(str[i]))
-   return AE_BAD_PARAMETER;
-   }
-   for (i = 0; i < 16; i++) {
-   uuid[i] =