Re: [PATCH v2] ucc_uart: add support for Freescale QUICCEngine UART

[Timur Tabi <timur@freescale.com>]

Anton Vorontsov wrote:

>> +		ucc@2400 {
>> +			device_type = "serial";
>> +			compatible = "ucc_uart";
>> +			model = "UCC";
> 
> model isn't used, is it needed at all?

I have no idea, but all the other UCC nodes have it, so I'm going to 
keep it.  Maybe one day we'll merge QE and CPM drivers, so this would be 
useful.

>> +	if (soft_uart) {
>> +		struct qe_firmware_info *qe_fw_info;
>> +
>> +		qe_fw_info = qe_get_firmware_info();
>> +
>> +		/* Check if the firmware has been uploaded. */
>> +		if (strstr(qe_fw_info->id, "Soft-UART")) {
> 
> qe_fw_info used w/o NULL checking.

Ok.

> [...]
>> +
>> +	qe_port->port.irq = irq_of_parse_and_map(np, 0);
>> +	if (qe_port->port.irq == NO_IRQ) {
>> +		dev_err(&ofdev->dev, "could not map IRQ for UCC%u\n",
>> +		       qe_port->ucc_num + 1);
>> +		kfree(qe_port);
>> +		return -EINVAL;
>> +	}
>> +
>> +	np = of_find_node_by_type(NULL, "qe");
> 
> Please, add "fsl,qe" compatible matching.

Ok.

>> +static struct of_platform_driver ucc_uart_of_driver = {
>> +	.owner  	= THIS_MODULE,
>> +	.name   	= "ucc_uart",
> 
> Maybe better fsl,ucc_uart?

The CPM serial driver uses "cpm_uart" and the QE ethernet driver uses 
"ucc_geth", so ucc_uart matches the pattern.