Hi,Alan,
Thank for your suggestion for two point, I have updated the patch in other email.
-----Original Message-----
From: Alan Cox [mailto:alan@xxxxxxxxxxxxxxxxxxx]
Sent: Tuesday, November 06, 2012 5:46 PM
To: Chen, Jun D
Cc: Alan Cox; serial; Gorby, Russ; Bi, Chao; Liu, Chuansheng
Subject: Re: [PATCH] serial: ifx6x60: Add module parameters to manage the modem status through sysfs.
> dev_warn(&ifx_dev->spi_dev->dev, "*** SPI Timeout ***");
> + saved_ifx_dev->hangup_reasons |= HU_TIMEOUT;
> ifx_spi_ttyhangup(ifx_dev);
This one should be using ifx_dev->hangup_reasons |=
this part of the code is clean for multiple devices so the ifx_dev pointer should be used (it will always be the same as saved_ifx_dev currently so it's just a tidy up)
> +/**
> + * reset_modem - modem reset command function
> + * @val: a reference to the string where the modem reset query is
> +given
> + * @kp: an unused reference to the kernel parameter */
> +
> +static int reset_modem(const char *val, const struct kernel_param
> +*kp) {
> + unsigned long reset_request;
> + int err = 0;
> +
> + u16 addr = V1P35CNT_W;
> + u8 data, def_value;
> +
> + if (kstrtoul(val, 10, &reset_request) < 0)
> + return -EINVAL;
> +
> + if (!saved_ifx_dev) {
> + dev_dbg(&saved_ifx_dev->spi_dev->dev,
> + "%s is called before probe\n", __func__);
> + return -ENODEV;
> + }
> +
> + dev_dbg(&saved_ifx_dev->spi_dev->dev,
> + "%s requested !\n", __func__);
> +
> + if (test_and_set_bit(MR_START, &saved_ifx_dev->mdm_reset_state))
> + goto out;
This case returns 0 - rather than an error like -EBUSY - is that intentional ?
Otherwise
Acked-by: Alan Cox <alan@xxxxxxxxxxxxxxx>
and the two points noted can be fixed up with a later patch IMHO.
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
