linux-kernel - Re: [PATCH] serial/efm32: add new driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <201201251656.55514.arnd@arndb.de>
Date:	Wed, 25 Jan 2012 16:56:55 +0000
From:	Arnd Bergmann <arnd@...db.de>
To:	"Uwe Kleine-König" 
	<u.kleine-koenig@...gutronix.de>
Cc:	linux-kernel@...r.kernel.org, Alan Cox <alan@...ux.intel.com>,
	devicetree-discuss@...ts.ozlabs.org,
	"Greg Kroah-Hartman" <gregkh@...e.de>, kernel@...gutronix.de,
	linux-serial@...r.kernel.org
Subject: Re: [PATCH] serial/efm32: add new driver

On Monday 09 January 2012, Uwe Kleine-König wrote:
> On Fri, Dec 23, 2011 at 09:44:28PM +0100, Uwe Kleine-König wrote:
>  
> > > I would suggest that you also support the "clock-frequency" and/or
> > > "current-speed" properties that are defined for serial ports, see the
> > > of_serial driver.
> > I will have a look to find out what they mean and update the patch
> > accordingly.
> I took that look and I don't understand what they are good for in my
> case.
> clock-frequency is used to initialize port->uartclk (and helps setting
> up port->custom_divisor if current-speed is given). The driver I posted
> uses the clk API to determine the clock frequency. So shouldn't the frequency
> better be specified in the clk part of the dt? (I don't know yet how a
> dt binding for clks should/can look like, so this is a bit speculative,
> but I'd expect to have nothing more clk related in a device
> specification but a reference to a clk definition.)

The binding for 8250 serial ports is documented at
http://www.openfirmware.org/1275/bindings/devices/html/serial.html

If you can always use the clk API to find out the base clock rate,
that's probably fine, I was mostly trying to make sure we don't
introduce another duplicate API for this.

The "current-speed" property is used to describe the baud rate that
should be used by the kernel for this port in order to talk to
devices connected to the port. This is very useful if you need to
connect the boot console to a fixed-rate device, and would get
used instead of the 115200 default you have when nothing else
is configured on the command line. I don't really know why
there is no respective option to set parity or flow control

> Independant of my driver I wonder if defining current-speed should also
> result in
> 
> 	port->flags |= UPF_SPD_CUST;
> 
> (in of_platform_serial_setup()).

I believe that is not needed because you don't call uart_get_divisor,
which would be the only place that looks at this flag.

> Having said that and taking into account that my driver doesn't use
> port->custom_divisor, do you keep suggesting that I should use
> "clock-frequency" and/or "current-speed"?

clock-frequency seems to be unnecessary, but current-speed would
still make sense. custom_divisor is simply the method that of_serial
uses to communicate the bit rate to the 8250 base driver, but you
could set the divisor directly.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/