Re: [PATCH 3/5] usb: phy: Add AM335x PHY driver

[George Cherian <george.cherian@ti.com>]

On 7/31/2013 1:46 AM, Sebastian Andrzej Siewior wrote:
> This driver is a redo of my earlier attempt. It uses parts of the
> generic PHY driver and uses the new control driver for the register
> the phy needs to power on/off the phy. It also enables easy access for
> the wakeup register which is not yet implemented.
> The difference between the omap attempt is:
> - no static holding variable
> - one global visible function which exports a struct with callbacks to
>    access the "control" registers.
>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
>   

<snip>

> +static const struct of_device_id omap_control_usb_id_table[] = {
> +	{ .compatible = "ti,am335x-ctrl-module", .data = &ctrl_am335x },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, omap_control_usb_id_table);
> +
> +static struct platform_driver am335x_control_driver;
> +static int match(struct device *dev, void *data)
> +{
> +	struct device_node *node = (struct device_node *)data;
> +	return dev->of_node == node &&
> +		dev->driver == &am335x_control_driver.driver;
> +}
> +
> +struct phy_control *am335x_get_phy_control(struct device *dev)
> +{
> +	struct device_node *node;
> +	struct am335x_control_usb *ctrl_usb;
> +
> +	node = of_parse_phandle(dev->of_node, "ti,ctrl_mod", 0);
> +	if (!node)
> +		return NULL;
> +
> +	dev = bus_find_device(&platform_bus_type, NULL, node, match);

of_find_device_by_node doesn't work???

> +	ctrl_usb = dev_get_drvdata(dev);
> +	if (!ctrl_usb)
> +		return NULL;
> +	return &ctrl_usb->phy_ctrl;
> +}
> +EXPORT_SYMBOL_GPL(am335x_get_phy_control);
> +
> +static int am335x_control_usb_probe(struct platform_device *pdev)
> +{
> +	struct resource	*res;
> +	struct am335x_control_usb *ctrl_usb;
> +	const struct of_device_id *of_id;
> +	const struct phy_control *phy_ctrl;
> +
> +	of_id = of_match_node(omap_control_usb_id_table, pdev->dev.of_node);
> +	if (!of_id)
> +		return -EINVAL;
> +
> +	phy_ctrl = of_id->data;
> +
> +	ctrl_usb = devm_kzalloc(&pdev->dev, sizeof(*ctrl_usb), GFP_KERNEL);
> +	if (!ctrl_usb) {
> +		dev_err(&pdev->dev, "unable to alloc memory for control usb\n");
> +		return -ENOMEM;
> +	}
> +
> +	ctrl_usb->dev = &pdev->dev;
> +
> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "control_dev");
> +	ctrl_usb->reset_reg = devm_ioremap_resource(&pdev->dev, res);

As per current DTSI in patch 5 you are mapping from control module base till 0x650.
Cant it be split to 2 registers one mapping for phy on/off and one for phy wkup?

> +	if (IS_ERR(ctrl_usb->reset_reg))
> +		return PTR_ERR(ctrl_usb->reset_reg);
> +	spin_lock_init(&ctrl_usb->lock);
> +	ctrl_usb->phy_ctrl = *phy_ctrl;
> +
> +	dev_set_drvdata(ctrl_usb->dev, ctrl_usb);
> +	return 0;
> +}
> +
> +static struct platform_driver am335x_control_driver = {
> +	.probe		= am335x_control_usb_probe,
> +	.driver		= {
> +		.name	= "am335x-control-usb",
> +		.owner	= THIS_MODULE,
> +		.of_match_table = of_match_ptr(omap_control_usb_id_table),
> +	},
> +};
> +
> +module_platform_driver(am335x_control_driver);
> +MODULE_LICENSE("GPL v2");
>

-- 
-George