Re: [PATCH v10 2/2] tty/serial: Add Spreadtrum sc9836-uart driver support

[Peter Hurley]

On 01/29/2015 11:04 AM, Varka Bhadram wrote:
> Hi Peter,
> 
> On Thursday 29 January 2015 09:19 PM, Peter Hurley wrote:
>> Hi Varka,
>>
>> On 01/29/2015 10:26 AM, Varka Bhadram wrote:
>>> Hi,
>>>
>>> On Wednesday 28 January 2015 04:38 PM, Chunyan Zhang wrote:
>>>> Add a full sc9836-uart driver for SC9836 SoC which is based on the
>>>> spreadtrum sharkl64 platform.
>>>> This driver also support earlycon.
>>>>
>>>> Originally-by: Lanqing Liu <lanqing.liu@xxxxxxxxxxxxxx>
>>>> Signed-off-by: Orson Zhai <orson.zhai@xxxxxxxxxxxxxx>
>>>> Signed-off-by: Chunyan Zhang <chunyan.zhang@xxxxxxxxxxxxxx>
>>>> Acked-by: Arnd Bergmann <arnd@xxxxxxxx>
>>>> Reviewed-by: Peter Hurley <peter@xxxxxxxxxxxxxxxxxx>
>>>> ---
>>>>   drivers/tty/serial/Kconfig       |   18 +
>>>>   drivers/tty/serial/Makefile      |    1 +
>>>>   drivers/tty/serial/sprd_serial.c |  793 ++++++++++++++++++++++++++++++++++++++
>>>>   include/uapi/linux/serial_core.h |    3 +
>>>>   4 files changed, 815 insertions(+)
>>>>   create mode 100644 drivers/tty/serial/sprd_serial.c
>>>>
>>> (...)
>>>
>>>> +static int sprd_probe(struct platform_device *pdev)
>>>> +{
>>>> +    struct resource *res;
>>>> +    struct uart_port *up;
>>>> +    struct clk *clk;
>>>> +    int irq;
>>>> +    int index;
>>>> +    int ret;
>>>> +
>>>> +    for (index = 0; index < ARRAY_SIZE(sprd_port); index++)
>>>> +        if (sprd_port[index] == NULL)
>>>> +            break;
>>>> +
>>>> +    if (index == ARRAY_SIZE(sprd_port))
>>>> +        return -EBUSY;
>>>> +
>>>> +    index = sprd_probe_dt_alias(index, &pdev->dev);
>>>> +
>>>> +    sprd_port[index] = devm_kzalloc(&pdev->dev,
>>>> +        sizeof(*sprd_port[index]), GFP_KERNEL);
>>>> +    if (!sprd_port[index])
>>>> +        return -ENOMEM;
>>>> +
>>>> +    up = &sprd_port[index]->port;
>>>> +    up->dev = &pdev->dev;
>>>> +    up->line = index;
>>>> +    up->type = PORT_SPRD;
>>>> +    up->iotype = SERIAL_IO_PORT;
>>>> +    up->uartclk = SPRD_DEF_RATE;
>>>> +    up->fifosize = SPRD_FIFO_SIZE;
>>>> +    up->ops = &serial_sprd_ops;
>>>> +    up->flags = UPF_BOOT_AUTOCONF;
>>>> +
>>>> +    clk = devm_clk_get(&pdev->dev, NULL);
>>>> +    if (!IS_ERR(clk))
>>>> +        up->uartclk = clk_get_rate(clk);
>>>> +
>>>> +    res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>>> +    if (!res) {
>>>> +        dev_err(&pdev->dev, "not provide mem resource\n");
>>>> +        return -ENODEV;
>>>> +    }
>>> This check is not required. It will be done by devm_ioremap_resource()
>> I disagree. devm_ioremap_resource() interprets the NULL resource as
>> a bad parameter and returns -EINVAL which is then forwarded as the
>> return value from the probe.
>>
>> -ENODEV is the correct return value from the probe if the expected
>> resource is not available (either because it doesn't exist or was already
>> claimed by another driver).
> 
> Check on the resource happening with evm_ioremap_resource.
> 
> Not necessary to check multiple times.
> 
> I did series for all the drivers. see [1]
> 
> [1]: https://lkml.org/lkml/2014/11/3/986 <https://lkml.org/lkml/2014/11/3/986>

That's a usb series. Did you do a serial driver series I missed?
I don't see anything related in Greg's tty-next tree...

Regards,
Peter Hurley

--
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/