On Tuesday, November 12, 2013 09:14:45 PM Mika Westerberg wrote: > On Tue, Nov 12, 2013 at 05:26:39PM +0000, Anderson, Brandon wrote: > > > > > > -----Original Message----- > > From: Mika Westerberg [mailto:mika.westerberg@xxxxxxxxxxxxxxx] > > Sent: Tuesday, November 12, 2013 4:43 AM > > To: Anderson, Brandon > > Cc: ssg.sos.patches; linaro-acpi@xxxxxxxxxxxxxxxx; linux-acpi@xxxxxxxxxxxxxxx > > Subject: Re: [PATCH 3/4] ACPI/ARM: Add ACPI to AMBA SPI driver > > > > On Mon, Nov 11, 2013 at 11:16:09AM -0600, Brandon Anderson wrote: > > > +#ifdef CONFIG_ACPI > > > +static struct pl022_ssp_controller * > > > +acpi_pl022_get_platform_data(struct device *dev) { > > > + struct pl022_ssp_controller *pd, *ret; > > > + struct acpi_amba_dsm_entry entry; > > > + > > > + pd = devm_kzalloc(dev, sizeof(struct pl022_ssp_controller), GFP_KERNEL); > > > + if (!pd) { > > > + dev_err(dev, "cannot allocate platform data memory\n"); > > > + return NULL; > > > + } > > > + ret = pd; > > > + > > > + pd->bus_id = -1; > > > + pd->enable_dma = 1; > > > + if (acpi_amba_dsm_lookup(ACPI_HANDLE(dev), "num-cs", 0, &entry) == 0) { > > > + if (kstrtou8(entry.value, 0, &pd->num_chipselect) != 0) { > > > + dev_err(dev, "invalid 'num-cs' in ACPI definition\n"); > > > + ret = NULL; > > > + } > > > + kfree(entry.key); > > > + kfree(entry.value); > > > + } > > > + if (acpi_amba_dsm_lookup(ACPI_HANDLE(dev), > > > + "autosuspend-delay", 0, &entry) == 0) { > > > + if (kstrtoint(entry.value, 0, &pd->autosuspend_delay) != 0) { > > > + dev_err(dev, "invalid 'autosuspend-delay' in ACPI definition\n"); > > > + ret = NULL; > > > + } > > > + kfree(entry.key); > > > + kfree(entry.value); > > > + } > > > + if (acpi_amba_dsm_lookup(ACPI_HANDLE(dev), "rt", 0, &entry) == 0) { > > > + pd->rt = (entry.value && strcmp(entry.value, "1") == 0); > > > + kfree(entry.key); > > > + kfree(entry.value); > > > + } > > > + > > > + return ret; > > > +} > > > +#endif > > > > If you add dummy stub acpi_pl022_get_platform_data() here in case of !CONFIG_ACPI... > > > > > + > > > static int pl022_probe(struct amba_device *adev, const struct amba_id > > > *id) { > > > struct device *dev = &adev->dev; > > > @@ -2081,6 +2125,11 @@ static int pl022_probe(struct amba_device > > > *adev, const struct amba_id *id) > > > > > > dev_info(&adev->dev, > > > "ARM PL022 driver, device ID: 0x%08x\n", adev->periphid); > > > +#ifdef CONFIG_ACPI > > > > You don't need this ugly #ifdef here. > > > > > > Ok, I understand the alternative. Is one way or the other required for > > patch acceptance? > > I guess it is up to the maintainer to decide in the end. Well, if I were the maintainer in question, I'd ask that do be done the way Mika suggested. The same applies if my ACK under the patch is requisite (which I believe should be the case here). Thanks! -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
