Messages in this thread | | | Subject | Re: [PATCH V1 3/4] usb: serial: f81534: add output pin control | From | "Ji-Ze Hong (Peter Hong)" <> | Date | Thu, 21 Dec 2017 17:49:45 +0800 |
| |
Hi Johan,
Johan Hovold 於 2017/12/19 上午 12:06 寫道: > On Thu, Nov 16, 2017 at 03:46:08PM +0800, Ji-Ze Hong (Peter Hong) wrote: >> +static int f81534_set_port_output_pin(struct usb_serial_port *port) >> +{ >> + struct f81534_serial_private *serial_priv; >> + struct f81534_port_private *port_priv; >> + struct usb_serial *serial; >> + const struct f81534_port_out_pin *pins; >> + int status; >> + int i; >> + u8 value; >> + u8 idx; >> + >> + serial = port->serial; >> + serial_priv = usb_get_serial_data(serial); >> + port_priv = usb_get_serial_port_data(port); >> + >> + idx = F81534_CONF_GPIO_OFFSET + port_priv->phy_num; >> + value = serial_priv->conf_data[idx]; >> + pins = &f81534_port_out_pins[port_priv->phy_num]; >> + >> + for (i = 0; i < ARRAY_SIZE(pins->pin); ++i) { >> + status = f81534_set_mask_register(serial, >> + pins->pin[i].reg_addr, pins->pin[i].reg_mask, >> + value & BIT(i) ? pins->pin[i].reg_mask : 0); >> + if (status) >> + return status; >> + } > > You're using 24 (get or set) accesses to update these three registers > here. Why not read them out (if necessary), determine their new values > and then write them back when done instead? >
In this code, I'm only read/write 3 registers of 0x2ae8, 0x2a90, 0x2a80, but some register will read/write more than once. Should I change the code from port_probe() to attach() and re-write it as: 1: read the 3 register 2: change them will 12 pin desire value 3: write it back Is it ok?
Thanks -- With Best Regards, Peter Hong
| |