On Thu, Jun 15, 2017 at 02:59:35PM -0500, Ian Pilcher wrote:
> On 06/15/2017 01:58 AM, Jiahau Chang wrote:
> > +void usb_asmedia_modifyflowcontrol(struct pci_dev *pdev)
> > +{
> > + unsigned char value;
> > + unsigned long wait_time_count;
> > +
> > + /*check device can accept command*/
> > + wait_time_count = 1000;
> > + while (wait_time_count) {
> > + pci_read_config_byte(pdev, ASMT_CONTROL_REG, &value);
> > + if (value == 0xff) {
> > + dev_dbg(&pdev->dev, "%s: check_ready ERROR", __func__);
> > + goto err_exit;
> > + }
> > + if ((value & ASMT_CONTROL_WRITE_BIT) == 0)
> > + break;
> > + wait_time_count--;
> > + usleep(50);
> > + }
> > + if (wait_time_count == 0) {
> > + dev_dbg(&pdev->dev, "%s: check_write_ready timeout", __func__);
> > + goto err_exit;
> > + }
> > + /* send command and address to device */
> > + pci_write_config_dword(pdev, ASMT_DATA_WRITE0_REG, ASMT_WRITEREG_CMD);
> > + pci_write_config_dword(pdev, ASMT_DATA_WRITE1_REG, ASMT_FLOWCTL_ADDR);
> > + pci_write_config_byte(pdev, ASMT_CONTROL_REG, ASMT_CONTROL_WRITE_BIT);
> > + wait_time_count = 1000;
> > + /* wait device receive the data */
> > + while (wait_time_count) {
> > + pci_read_config_byte(pdev, ASMT_CONTROL_REG, &value);
> > + if (value == 0xff) {
> > + dev_dbg(&pdev->dev, "%s: wait_ready ERROR", __func__);
> > + goto err_exit;
> > + }
> > + if ((value & ASMT_CONTROL_WRITE_BIT) == 0)
> > + break;
> > + wait_time_count--;
> > + usleep(50);
> > + }
> > + if (wait_time_count == 0) {
> > + dev_dbg(&pdev->dev, "%s: wait_write_ready timeout", __func__);
> > + goto err_exit;
> > + }
> > + /* send data to device */
> > + pci_write_config_dword(pdev, ASMT_DATA_WRITE0_REG, ASMT_FLOWCTL_DATA);
> > + pci_write_config_dword(pdev, ASMT_DATA_WRITE1_REG, ASMT_PSEUDO_DATA);
> > + pci_write_config_byte(pdev, ASMT_CONTROL_REG, ASMT_CONTROL_WRITE_BIT);
> > +err_exit:
> > + return;
> > +}
>
> As others have pointed out, this flow is not very clear to the first-
> time reader. Maybe something like this (WARNING untested):
>
> static int usb_asmedia_wait_write(struct pci_dev *pdev, unsigned long
> wait_usec)
wait_usec is a count here:
> {
> unsigned long wait_count;
> unsigned char value;
>
> for (wait_count = wait_usec / 50; wait_count > 0; --wait_count) {
>
> pci_read_config_byte(pdev, ASMT_CONTROL_REG, &value);
>
> if (value == 0xff) {
> dev_dbg(&pdev->dev, "%s: check_ready ERROR", __func__);
> return -1;
> }
>
> if ((value & ASMT_CONTROL_WRITE_BIT) == 0)
> return 0;
You aren't sleeping any :(
> }
>
> dev_dbg(&pdev->dev, "%s: check_write_ready timeout", __func__);
> return -1;
Always return real error codes, never make up random numbers.
thanks,
greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
