Re: [PATCH v4] Bluetooth: Add hci_h4p driver

From: Sebastian Reichel
Date: Thu Jan 02 2014 - 20:05:27 EST


Hi,

On Fri, Jan 03, 2014 at 01:17:54AM +0100, Pavel Machek wrote:
> Changes from v3: Moved platform data into
> include/linux/platform_data/, something I missed before.

As I wrote before Tony plans to remove the boardcode for all
OMAP boards including the Nokia N900 for 3.14, so you cannot
boot without DT from 3.14 onwards.

The drivers can still be initialized the old way using pdata quirks
until all drivers are converted, but I think this driver can simply
be prepared for DT directly:

> [...]
>
> +struct hci_h4p_platform_data {
> + int chip_type;

This can be "extracted" from the compatible string.

> + int bt_sysclk;

This can be converted into a vendor property.

> + unsigned int bt_wakeup_gpio;
> + unsigned int host_wakeup_gpio;
> + unsigned int reset_gpio;

These can easily be acquired via DT.

> + int reset_gpio_shared;

This looks like a simple property in the DT structure.

You should use a boolean type for this btw.

> + unsigned int uart_irq;

This one can also simply be aquired via DT.

> + phys_addr_t uart_base;

I see multiple ways for this one:

1. Just put the memory address into the dts file.
2. Make this a phandle to the UART node and get
the memory address from the referenced node.
3. Make the bluetooth node a subnode of the UART
node and get the address from the parent node.

IMHO solution 3 is the best solution, since the bluetooth
chip is basically connected to the system via the UART.

> + const char *uart_iclk;
> + const char *uart_fclk;

There is currently work going on to move OMAP's clock
data into DT. When that work is done the clocks can
be acquired via phandles. I think it's expected to
be merged into 3.14.

> + void (*set_pm_limits)(struct device *dev, bool set);

If I'm not mistaken set_pm_limits is only referenced by
hci_h4p_set_pm_limits(). The hci_h4p_set_pm_limits()
function is not referenced anywhere, thus both can be
removed.

> +};

-- Sebastian

Attachment: signature.asc
Description: Digital signature