Hello Marek,

On Tue, Aug 14, 2012 at 2:50 PM, Marek Vasut [off-list ref] wrote:

Dear Tomas Hlavacek,

quoted

+static ssize_t get_attr_uartclk(struct device *dev,
+             struct device_attribute *attr, char *buf)
+{
+     int ret;
+
+     struct uart_state *state = (struct uart_state *)(dev_get_drvdata(dev));

You don't need this cast here.

Yes, you are right. Thanks.

quoted

+     mutex_lock(&state->port.mutex);
+     ret = snprintf(buf, PAGE_SIZE, "%d\n", state->uart_port->uartclk);

Do you really need such a large buffer (PAGE_SIZE) ?

Well no, but I believe that I get the buffer of length equal to
PAGE_SIZE anyway. Documentation/filesystems/sysfs.txt says so on line
179.

quoted

+     mutex_unlock(&state->port.mutex);
+     return ret;
+}
+
+static ssize_t set_attr_uartclk(struct device *dev,
+             struct device_attribute *attr, const char *buf, size_t count)
+{
+     struct uart_state *state = (struct uart_state *)(dev_get_drvdata(dev));
+     unsigned int val;
+     int ret;
+
+     ret = kstrtouint(buf, 10, &val);
+     if (ret)
+             return ret;
+
+     mutex_lock(&state->port.mutex);
+
+     /*
+      * Check value: baud_base has to be more than 9600
+      * and uartclock = baud_base * 16 .
+      */
+     if (val >= 153600)
+             state->uart_port->uartclk = val;

This magic value here would use some documentation.

OK. Do you think this would be sufficient comment?:

/*
 * Check value: baud_base does not make sense to be set below 9600
 * and since uartclock = (baud_base * 16) it has to be equal or greater than
 * 9600 * 16 = 153600.
 */

PATCHv2 follows.

Tomas


-- 
Tomáš Hlaváček [off-list ref]