Re: [PATCH 3/3] acpi:lpss:Refine lpss codes to fit acpi soc.

[Ken Xue]

On Tue, 2014-12-16 at 12:16 +0200, Mika Westerberg wrote:
> On Thu, Dec 11, 2014 at 11:05:51AM +0800, Ken Xue wrote:
> 
> This is missing changelog.
> 
> > Signed-off-by: Ken Xue <Ken.Xue@xxxxxxx>
> 
> ...
> 
[Ken]got it.

> > +static int lpss_common_setup(struct acpi_soc_dev_private_data *pdata)
> >  {
> > -	struct lpss_device_desc *dev_desc;
> > -	struct lpss_private_data *pdata;
> > -	struct resource_list_entry *rentry;
> > -	struct list_head resource_list;
> > -	struct platform_device *pdev;
> > -	int ret;
> > -
> > -	dev_desc = (struct lpss_device_desc *)id->driver_data;
> > -	if (!dev_desc) {
> > -		pdev = acpi_create_platform_device(adev);
> > -		return IS_ERR_OR_NULL(pdev) ? PTR_ERR(pdev) : 1;
> > -	}
> > -	pdata = kzalloc(sizeof(*pdata), GFP_KERNEL);
> > -	if (!pdata)
> > -		return -ENOMEM;
> > -
> > -	INIT_LIST_HEAD(&resource_list);
> > -	ret = acpi_dev_get_resources(adev, &resource_list, is_memory, NULL);
> > -	if (ret < 0)
> > -		goto err_out;
> > -
> > -	list_for_each_entry(rentry, &resource_list, node)
> > -		if (resource_type(&rentry->res) == IORESOURCE_MEM) {
> > -			if (dev_desc->prv_size_override)
> > -				pdata->mmio_size = dev_desc->prv_size_override;
> > -			else
> > -				pdata->mmio_size = resource_size(&rentry->res);
> > -			pdata->mmio_base = ioremap(rentry->res.start,
> > -						   pdata->mmio_size);
> > -			break;
> > -		}
> > -
> > -	acpi_dev_free_resource_list(&resource_list);
> > +	int ret = 0;
> > +	struct acpi_soc_dev_desc *dev_desc;
> 
> 
> Please order these so that the longest line is on top. E.g
> 
[ken]got it.

> 	struct acpi_soc_dev_desc *dev_desc;
> 	int ret = 0;
> 
> Ditto for other similar places.
> 
> >  
> > -	pdata->dev_desc = dev_desc;
> > +	dev_desc = pdata->dev_desc;
> >  
> > -	if (dev_desc->setup)
> > -		dev_desc->setup(pdata);
> > +	if (dev_desc->flags & LPSS_CLK)
> > +		ret = register_device_clock(pdata->adev, pdata);
> >  
> > -	if (dev_desc->flags & LPSS_CLK) {
> > -		ret = register_device_clock(adev, pdata);
> > -		if (ret) {
> > -			/* Skip the device, but continue the namespace scan. */
> > -			ret = 0;
> > -			goto err_out;
> > -		}
> > -	}
> > +	return ret;
> > +}
> 
> ...
> 
> >  void __init acpi_lpss_init(void)
> >  {
> >  	if (!lpt_clk_init()) {
> > -		bus_register_notifier(&platform_bus_type, &acpi_lpss_nb);
> > -		acpi_scan_add_handler(&lpss_handler);
> > +		a_soc.ids = acpi_lpss_device_ids;
> > +		a_soc.attr_group = &lpss_attr_group;
> > +		a_soc.pm_domain = &acpi_lpss_pm_domain;
> 
> If AMD stuff is not needing PM domain or LTR, I suggest that you leave
> both to be part of LPSS and not move them to acpi_soc.
> 
[Ken]there is no strong reason i can see to remove it. AMD may be also
has similar good feature in the future. and intel also can modify acpi
soc as you need, if it really can not meet your design. 

> > +		register_acpi_soc(&a_soc, true);
>                                           ^^^^ This needs to be false
> 
> >  	}
> >  }


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/