From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932645AbdERQkG (ORCPT ); Thu, 18 May 2017 12:40:06 -0400 Received: from david.siemens.de ([192.35.17.14]:36209 "EHLO david.siemens.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932254AbdERQkD (ORCPT ); Thu, 18 May 2017 12:40:03 -0400 Subject: Re: [PATCH v2 6/6] serial: exar: Add support for IOT2040 device To: Andy Shevchenko Cc: Greg Kroah-Hartman , Linus Walleij , Alexandre Courbot , Linux Kernel Mailing List , "linux-serial@vger.kernel.org" , "linux-gpio@vger.kernel.org" , Sudip Mukherjee , Sascha Weisenberger References: From: Jan Kiszka Message-ID: <65189e05-e300-bbe3-660d-4fdd6d4ec8c1@siemens.com> Date: Thu, 18 May 2017 18:39:50 +0200 User-Agent: Mozilla/5.0 (X11; U; Linux i686 (x86_64); de; rv:1.8.1.12) Gecko/20080226 SUSE/2.0.0.12-1.1 Thunderbird/2.0.0.12 Mnenhy/0.7.5.666 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2017-05-18 18:33, Andy Shevchenko wrote: > On Thu, May 18, 2017 at 5:59 PM, Jan Kiszka wrote: >> This implements the setup of RS232 and the switch-over to RS485 or RS422 >> for the Siemens IOT2040. That uses an EXAR XR17V352 with external logic >> to switch between the different modes. The external logic is controlled >> via MPIO pins of the EXAR controller. >> >> Only pin 10 can be exported as GPIO on the IOT2040. It is connected to >> an LED. >> >> As the XR17V352 used on the IOT2040 is not equipped with an external >> EEPROM, it cannot present itself as IOT2040-variant via subvendor/ >> subdevice IDs. Thus, we have to check via DMI for the target platform. >> >> Co-developed with Sascha Weisenberger. > > Few nits below and one comment that should be addressed. > >> +#define UART_EXAR_RS485_DLY(x) (x << 4) > > ((x) << 4) Yep. > >> +static bool is_iot2040; > > No, please, use driver data of DMI and hide this in corresponding structure. > Or even assign port->port.rs485_config in the callback function. > > Moreover, can't you use port->port.rs485_config != NULL instead? There are two cases to be handled on IOT2040: the setting of the rs485_config and the different setup of the GPIOs, but the latter at a specific point in the initialization only. So I don't see yet how driver_data could come into play and help. > >> + >> struct exar8250; >> >> /** >> @@ -212,6 +252,82 @@ xr17v35x_register_gpio(struct pci_dev *pcidev, unsigned int first_gpio, >> return pdev; >> } >> >> +static int iot2040_rs485_config(struct uart_port *port, >> + struct serial_rs485 *rs485) >> +{ >> + u8 __iomem *p = port->membase; >> + u8 mask = IOT2040_UART1_MASK; >> + u8 mode, value; > >> + bool is_rs485 = false; >> + >> + if (rs485->flags & SER_RS485_ENABLED) { >> + is_rs485 = true; > > bool is_rs485 = !!(rs485->flags & SER_RS485_ENABLED); > > if (is_rs485) { > ... > } else { > ... > } > OK. >> + return 0; >> +} > >> +static int iot2040_match(const struct dmi_system_id *dmi) >> +{ >> + is_iot2040 = true; >> + return 1; >> +} > > See above. See above. Jan > >> + >> +static const struct dmi_system_id exar_platforms[] = { >> + { >> + .callback = iot2040_match, >> + .ident = "IOT2040", >> + .matches = { >> + DMI_MATCH(DMI_BOARD_NAME, "SIMATIC IOT2000"), >> + DMI_MATCH(DMI_BOARD_ASSET_TAG, "6ES7647-0AA00-1YA2"), >> + }, >> + } >> +}; >