| Message ID | 20220729003033.771761-1-utkarsh.h.patel@intel.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 1b1b672cc1d4fb3065dac79efb8901bd6244ef69 |
| Headers | show |
| Series | usb: typec: intel_pmc_mux: Add new ACPI ID for Meteor Lake IOM device | expand |
On Thu, Jul 28, 2022 at 05:30:33PM -0700, Utkarsh Patel wrote: > Intel Meteor Lake IOM uses 64bit IOM BASE address than previous Intel > Generations which use 32bit. I can not parse this sentence, sorry. > Added code to support 64bit IOM BASE address change with necessary ACPI > resource extraction support. Again, I do not understand :( > > Signed-off-by: Utkarsh Patel <utkarsh.h.patel@intel.com> > Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> No need to backport this to stable kernels? > --- > drivers/usb/typec/mux/intel_pmc_mux.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/drivers/usb/typec/mux/intel_pmc_mux.c b/drivers/usb/typec/mux/intel_pmc_mux.c > index 47b733f78fb0..a8e273fe204a 100644 > --- a/drivers/usb/typec/mux/intel_pmc_mux.c > +++ b/drivers/usb/typec/mux/intel_pmc_mux.c > @@ -571,9 +571,11 @@ static int pmc_usb_register_port(struct pmc_usb *pmc, int index, > > static int is_memory(struct acpi_resource *res, void *data) > { > - struct resource r; > + struct resource_win win = {}; > + struct resource *r = &win.res; > > - return !acpi_dev_resource_memory(res, &r); > + return !(acpi_dev_resource_memory(res, r) || > + acpi_dev_resource_address_space(res, &win)); I don't understand, what is the extra check here doing? Why is that needed and why aren't you triggering off of a platform type? thanks, greg k-h
Hi Utkarsh, On Thu, Jul 28, 2022 at 05:30:33PM -0700, Utkarsh Patel wrote: > Intel Meteor Lake IOM uses 64bit IOM BASE address than previous Intel > Generations which use 32bit. > > Added code to support 64bit IOM BASE address change with necessary ACPI > resource extraction support. > > Signed-off-by: Utkarsh Patel <utkarsh.h.patel@intel.com> > Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> > --- > drivers/usb/typec/mux/intel_pmc_mux.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/drivers/usb/typec/mux/intel_pmc_mux.c b/drivers/usb/typec/mux/intel_pmc_mux.c > index 47b733f78fb0..a8e273fe204a 100644 > --- a/drivers/usb/typec/mux/intel_pmc_mux.c > +++ b/drivers/usb/typec/mux/intel_pmc_mux.c > @@ -571,9 +571,11 @@ static int pmc_usb_register_port(struct pmc_usb *pmc, int index, > > static int is_memory(struct acpi_resource *res, void *data) > { > - struct resource r; > + struct resource_win win = {}; > + struct resource *r = &win.res; > > - return !acpi_dev_resource_memory(res, &r); > + return !(acpi_dev_resource_memory(res, r) || > + acpi_dev_resource_address_space(res, &win)); > } I realised that now that is_memory() function is basically just a copy of the is_memory() function that's in drivers/acpi/resources.c, so I think we need to handle this a bit differently. There are a few places in kernel that have that same check. One way would be to just export the is_memory() function that's in drivers/acpi/resources.c, but since we have already a wrapper function acpi_dev_get_dma_resources() for DMA resouces, I think we could have a similar wrapper for common memory resources. I'll prepare a patch(s) where I'll propose a new wrapper function acpi_dev_get_memory_resources() that will take care of the is_memory() check, and then convert the users (including this driver). After that, this patch only needs to add the ID. thanks, > /* IOM ACPI IDs and IOM_PORT_STATUS_OFFSET */ > @@ -583,6 +585,9 @@ static const struct acpi_device_id iom_acpi_ids[] = { > > /* AlderLake */ > { "INTC1079", 0x160, }, > + > + /* Meteor Lake */ > + { "INTC107A", 0x160, }, > {} > }; > > -- > 2.25.1
Hi Heikki, Thank you for the review and feedback. > > drivers/usb/typec/mux/intel_pmc_mux.c | 9 +++++++-- > > 1 file changed, 7 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/usb/typec/mux/intel_pmc_mux.c > > b/drivers/usb/typec/mux/intel_pmc_mux.c > > index 47b733f78fb0..a8e273fe204a 100644 > > --- a/drivers/usb/typec/mux/intel_pmc_mux.c > > +++ b/drivers/usb/typec/mux/intel_pmc_mux.c > > @@ -571,9 +571,11 @@ static int pmc_usb_register_port(struct pmc_usb > > *pmc, int index, > > > > static int is_memory(struct acpi_resource *res, void *data) { > > - struct resource r; > > + struct resource_win win = {}; > > + struct resource *r = &win.res; > > > > - return !acpi_dev_resource_memory(res, &r); > > + return !(acpi_dev_resource_memory(res, r) || > > + acpi_dev_resource_address_space(res, &win)); > > } > > I realised that now that is_memory() function is basically just a copy of the > is_memory() function that's in drivers/acpi/resources.c, so I think we need to > handle this a bit differently. There are a few places in kernel that have that > same check. > > One way would be to just export the is_memory() function that's in > drivers/acpi/resources.c, but since we have already a wrapper function > acpi_dev_get_dma_resources() for DMA resouces, I think we could have a > similar wrapper for common memory resources. > > I'll prepare a patch(s) where I'll propose a new wrapper function > acpi_dev_get_memory_resources() that will take care of the is_memory() > check, and then convert the users (including this driver). After that, this patch > only needs to add the ID. I will wait for your changes then. Sincerely, Utkarsh Patel.
diff --git a/drivers/usb/typec/mux/intel_pmc_mux.c b/drivers/usb/typec/mux/intel_pmc_mux.c index 47b733f78fb0..a8e273fe204a 100644 --- a/drivers/usb/typec/mux/intel_pmc_mux.c +++ b/drivers/usb/typec/mux/intel_pmc_mux.c @@ -571,9 +571,11 @@ static int pmc_usb_register_port(struct pmc_usb *pmc, int index, static int is_memory(struct acpi_resource *res, void *data) { - struct resource r; + struct resource_win win = {}; + struct resource *r = &win.res; - return !acpi_dev_resource_memory(res, &r); + return !(acpi_dev_resource_memory(res, r) || + acpi_dev_resource_address_space(res, &win)); } /* IOM ACPI IDs and IOM_PORT_STATUS_OFFSET */ @@ -583,6 +585,9 @@ static const struct acpi_device_id iom_acpi_ids[] = { /* AlderLake */ { "INTC1079", 0x160, }, + + /* Meteor Lake */ + { "INTC107A", 0x160, }, {} };