Hi Chanwoo,

On 8/29/2013 5:42 PM, Chanwoo Choi wrote:
[big snip ]
I tested various development board based on Samsung Exynos series SoC.
Although some gpio of Exynos series SoC set high state(non zero, 1) as default 
value,
this gpio state could mean off state, disconnected or un-powered state 
according to gpio.
Of course, above explanation about specific gpio don't include all gpios.
This is meaning that there is possibility.
okay then how about adding a flag for inverted logic too.  something like this

if(of_property_read_bool(np,"inverted_gpio))
     gpio_usbvid->gpio_inv = 1;
And always check on this before deciding?
Is this fine ?

Second,
gpio_usbvid_set_initial_state() dertermine both "USB-HOST" and "USB" cable 
state at same time
in 'case ID_DETCT' according to 'gpio_usbvid->id_gpio'. But, I think that other 
extcon devices
would not control both "USB-HOST" and "USB" cable state at same time.

Other extcon devices would support either "USB-HOST" or "USB" cable.
The driver has 2 configurations.
1) supports implementations with both VBUS and ID pin are routed via gpio's for 
swicthing roles(compatible  gpio-usb-vid).
As you explained about case 1, it is only used on dra7x SoC.
No gpio-usb-id is used in dra7xx. dra7xx has got only ID pin routed via gpio.
Other SoC could not wish to control both "USB-HOST" and "USB" cable at same 
time.
I could'nt actually parse this. You meant since the id_irq_handler handles both USB and USB-HOST cable
its not proper?
I need your answer about above my opinion for other SoC.
So how about this, I have removed the dra7x specific stuffs (gpio-usb-id)

static void gpio_usbvid_set_initial_state(struct gpio_usbvid *gpio_usbvid)
{
        int id_current, vbus_current;

    id_current = gpio_get_value_cansleep(gpio_usbvid->id_gpio);
    if (!!id_current == ID_FLOAT)
        extcon_set_cable_state(&gpio_usbvid->edev, "USB-HOST", false);
    else
        extcon_set_cable_state(&gpio_usbvid->edev, "USB-HOST", true);

    vbus_current = gpio_get_value_cansleep(gpio_usbvid->vbus_gpio);
     if (!!vbus_current == VBUS_ON)
        extcon_set_cable_state(&gpio_usbvid->edev, "USB", true);
    else
        extcon_set_cable_state(&gpio_usbvid->edev, "USB", false);
}

and the irq handlers like this

static irqreturn_t id_irq_handler(int irq, void *data)
{
        struct gpio_usbvid *gpio_usbvid = (struct gpio_usbvid *)data;
        int id_current;

        id_current = gpio_get_value_cansleep(gpio_usbvid->id_gpio);
        if (id_current == ID_GND)
extcon_set_cable_state(&gpio_usbvid->edev, "USB-HOST", true);
        else
extcon_set_cable_state(&gpio_usbvid->edev, "USB-HOST", false);
        return IRQ_HANDLED;
}

static irqreturn_t vbus_irq_handler(int irq, void *data)
{
        struct gpio_usbvid *gpio_usbvid = (struct gpio_usbvid *)data;
        int vbus_current;

        vbus_current = gpio_get_value_cansleep(gpio_usbvid->vbus_gpio);
        if (vbus_current == VBUS_OFF)
                extcon_set_cable_state(&gpio_usbvid->edev, "USB", false);
        else
                extcon_set_cable_state(&gpio_usbvid->edev, "USB", true);

        return IRQ_HANDLED;
}
[snip]
I have some confusion. I need additional your explanation.
Could this driver register only one interrupt handler either id_irq_handler() 
or vbus_irq_handler()?
If compatible == ID_DETECT, only one handler --> id_irq_handler() and it will 
handle both USB and USB-HOST
or
Could this driver register two interrupt handler both id_irq_handler() or 
vbus_irq_handler()?
If compatible == VBUS_ID_DETECT, 2 handlers --> id_irq_handler() will handle 
USB-HOST and vbus_irq_handler will handle USB.
As you mentioned, we cannot only control either USB or USB-HOST cable on this 
extcon driver.
This extcon driver is only suitable dra7x SoC.
Do you still feel its dra7x specific if i modify it as above?
Because id_irq_handler() control both "USB-HOST" and "USB" cable at same time,
you need this setting order between "USB-HOST" and "USB" cable.
yes
I think that the setting order between cables isn't general. It is specific 
method for dra7x SoC.
So if i remove that part then?
Did you think that SoC should always connect either "USB-HOST" cable or "USB" 
cable?
No,  even if a physical cable is not connected it should  default to either 
USB-HOST or USB
It isn't general.

If physical cable isn't connected to extcon device, the state both USB-HOST and 
USB cable
should certainly be zero. Because The extcon consumer driver could set proper 
operation
according to cable state.
okay


I don't know this case except for dra7x SoC. So, I think it has dependency on 
specific SoC.
I need your answer about above my opinion.
Hope i could answer you :-)
and can't agree as generic extcon gpio driver.


--
-George

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to