On Sat, 2017-10-28 at 14:11 +0200, Wolfram Sang wrote: > Thanks for the patch! > > On Mon, Oct 23, 2017 at 03:12:20PM -0700, Hoan Tran wrote: > > This patch supports xgene-slimpro-i2c v2 which uses the non-cachable > > memory > > as the PCC shared memory. > > > > Signed-off-by: Hoan Tran <hotran@xxxxxxx> > > I can't tell if __force is justified here but I don't know much about > ACPI... Andy, can you have a look? Few comments to the patch as well. > > +#ifdef CONFIG_ACPI > > +static const struct acpi_device_id xgene_slimpro_i2c_acpi_ids[] = { > > + {"APMC0D40", XGENE_SLIMPRO_I2C_V1}, > > + {"APMC0D8B", XGENE_SLIMPRO_I2C_V2}, > > + {} > > +}; > > +MODULE_DEVICE_TABLE(acpi, xgene_slimpro_i2c_acpi_ids); > > +#endif No need to move this here... > > + > > static int xgene_slimpro_i2c_probe(struct platform_device *pdev) > > { > > struct slimpro_i2c_dev *ctx; > > @@ -476,6 +490,17 @@ static int xgene_slimpro_i2c_probe(struct > > platform_device *pdev) > > } > > } else { > > struct acpi_pcct_hw_reduced *cppc_ss; > > + int version = XGENE_SLIMPRO_I2C_V1; > > +#ifdef CONFIG_ACPI Do you really need this ifdef? > > + const struct acpi_device_id *acpi_id; > > + > > + acpi_id = > > acpi_match_device(xgene_slimpro_i2c_acpi_ids, ... you may get pointer to id table via pdev. > > + &pdev->dev); > > + if (!acpi_id) > > + return -EINVAL; > > + > > + version = (int)acpi_id->driver_data; > > +#endif > > if (ctx->comm_base_addr) { > > - ctx->pcc_comm_addr = memremap(ctx- > > >comm_base_addr, > > - cppc_ss- > > >length, > > - MEMREMAP_WB); > > + if (version == XGENE_SLIMPRO_I2C_V2) > > + ctx->pcc_comm_addr = (void __force > > *)ioremap( > > + ctx- > > >comm_base_addr, > > + cppc_ss- > > >length); Commit message actually doesn't explain why we switch to iomap over memmap. (__force here is obviously to suppress __iomap annotation for mapped memory) If you need non-cacheable MEMREMAP_WT I guess will do a job. > > + else > > + ctx->pcc_comm_addr = memremap( > > + ctx- > > >comm_base_addr, > > + cppc_ss- > > >length, > > + MEMREMAP_WB > > ); -- Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> Intel Finland Oy