From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752937AbbA2Pto (ORCPT ); Thu, 29 Jan 2015 10:49:44 -0500 Received: from mail-qg0-f52.google.com ([209.85.192.52]:50200 "EHLO mail-qg0-f52.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751448AbbA2Ptl (ORCPT ); Thu, 29 Jan 2015 10:49:41 -0500 Message-ID: <54CA568E.6080306@hurleysoftware.com> Date: Thu, 29 Jan 2015 10:49:34 -0500 From: Peter Hurley User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.4.0 MIME-Version: 1.0 To: Varka Bhadram , Chunyan Zhang , gregkh@linuxfoundation.org CC: robh+dt@kernel.org, mark.rutland@arm.com, arnd@arndb.de, gnomes@lxorguk.ukuu.org.uk, pawel.moll@arm.com, ijc+devicetree@hellion.org.uk, galak@codeaurora.org, grant.likely@linaro.org, jslaby@suse.cz, heiko@sntech.de, jason@lakedaemon.net, florian.vaussard@epfl.ch, andrew@lunn.ch, hytszk@gmail.com, antonynpavlov@gmail.com, shawn.guo@linaro.org, orsonzhai@gmail.com, geng.ren@spreadtrum.com, zhizhou.zhang@spreadtrum.com, lanqing.liu@spreadtrum.com, zhang.lyra@gmail.com, wei.qiao@spreadtrum.com, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-serial@vger.kernel.org, linux-api@vger.kernel.org, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH v10 2/2] tty/serial: Add Spreadtrum sc9836-uart driver support References: <1422443324-25082-1-git-send-email-chunyan.zhang@spreadtrum.com> <1422443324-25082-3-git-send-email-chunyan.zhang@spreadtrum.com> <54CA5128.8050500@gmail.com> In-Reply-To: <54CA5128.8050500@gmail.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 >> Signed-off-by: Orson Zhai >> Signed-off-by: Chunyan Zhang >> Acked-by: Arnd Bergmann >> Reviewed-by: Peter Hurley >> --- >> 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). Regards, Peter Hurley >> + up->mapbase = res->start; >> + up->membase = devm_ioremap_resource(&pdev->dev, res); >> + if (IS_ERR(up->membase)) >> + return PTR_ERR(up->membase); >> + >> > From mboxrd@z Thu Jan 1 00:00:00 1970 From: Peter Hurley Subject: Re: [PATCH v10 2/2] tty/serial: Add Spreadtrum sc9836-uart driver support Date: Thu, 29 Jan 2015 10:49:34 -0500 Message-ID: <54CA568E.6080306@hurleysoftware.com> References: <1422443324-25082-1-git-send-email-chunyan.zhang@spreadtrum.com> <1422443324-25082-3-git-send-email-chunyan.zhang@spreadtrum.com> <54CA5128.8050500@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <54CA5128.8050500-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> Sender: devicetree-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Varka Bhadram , Chunyan Zhang , gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org, mark.rutland-5wv7dgnIgG8@public.gmane.org, arnd-r2nGTMty4D4@public.gmane.org, gnomes-qBU/x9rampVanCEyBjwyrvXRex20P6io@public.gmane.org, pawel.moll-5wv7dgnIgG8@public.gmane.org, ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org, galak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org, grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org, jslaby-AlSwsSmVLrQ@public.gmane.org, heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org, jason-NLaQJdtUoK4Be96aLqz0jA@public.gmane.org, florian.vaussard-p8DiymsW2f8@public.gmane.org, andrew-g2DYL2Zd6BY@public.gmane.org, hytszk-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org, antonynpavlov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org, shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org, orsonzhai-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org, geng.ren-lxIno14LUO0EEoCn2XhGlw@public.gmane.org, zhizhou.zhang-lxIno14LUO0EEoCn2XhGlw@public.gmane.org, lanqing.liu-lxIno14LUO0EEoCn2XhGlw@public.gmane.org, zhang.lyra-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org, wei.qiao-lxIno14LUO0EEoCn2XhGlw@public.gmane.org, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-serial-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org List-Id: devicetree@vger.kernel.org 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 >> Signed-off-by: Orson Zhai >> Signed-off-by: Chunyan Zhang >> Acked-by: Arnd Bergmann >> Reviewed-by: Peter Hurley >> --- >> 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). Regards, Peter Hurley >> + up->mapbase = res->start; >> + up->membase = devm_ioremap_resource(&pdev->dev, res); >> + if (IS_ERR(up->membase)) >> + return PTR_ERR(up->membase); >> + >> > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html From mboxrd@z Thu Jan 1 00:00:00 1970 From: peter@hurleysoftware.com (Peter Hurley) Date: Thu, 29 Jan 2015 10:49:34 -0500 Subject: [PATCH v10 2/2] tty/serial: Add Spreadtrum sc9836-uart driver support In-Reply-To: <54CA5128.8050500@gmail.com> References: <1422443324-25082-1-git-send-email-chunyan.zhang@spreadtrum.com> <1422443324-25082-3-git-send-email-chunyan.zhang@spreadtrum.com> <54CA5128.8050500@gmail.com> Message-ID: <54CA568E.6080306@hurleysoftware.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org 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 >> Signed-off-by: Orson Zhai >> Signed-off-by: Chunyan Zhang >> Acked-by: Arnd Bergmann >> Reviewed-by: Peter Hurley >> --- >> 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). Regards, Peter Hurley >> + up->mapbase = res->start; >> + up->membase = devm_ioremap_resource(&pdev->dev, res); >> + if (IS_ERR(up->membase)) >> + return PTR_ERR(up->membase); >> + >> >