Re: [PATCH v6 2/2] HID: i2c-hid: elan: Add ili9882t timing

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On Mon, Aug 21, 2023 at 2:01 AM Benjamin Tissoires <bentiss@xxxxxxxxxx> wrote:
>
> On Aug 02 2023, Doug Anderson wrote:
> > Benjamin,
> >
> > On Wed, Aug 2, 2023 at 12:20 AM Cong Yang
> > <yangcong5@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxx> wrote:
> > >
> > > The ili9882t is a TDDI IC (Touch with Display Driver). The
> > > datasheet specifies there should be 60ms between touch SDA
> > > sleep and panel RESX. Doug's series[1] allows panels and
> > > touchscreens to power on/off together, so we can add the 65 ms
> > > delay in i2c_hid_core_suspend before panel_unprepare.
> > >
> > > Because ili9882t touchscrgeen is a panel follower, and
> > > needs to use vccio-supply instead of vcc33-supply, so set
> > > it NULL to ili9882t_chip_data, then not use vcc33 regulator.
> > >
> > > [1]: https://lore.kernel.org/all/20230727171750.633410-1-dianders@xxxxxxxxxxxx
> > >
> > > Reviewed-by: Douglas Anderson <dianders@xxxxxxxxxxxx>
> > > Signed-off-by: Cong Yang <yangcong5@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxx>
> > > ---
> > >  drivers/hid/i2c-hid/i2c-hid-of-elan.c | 50 ++++++++++++++++++++-------
> > >  1 file changed, 38 insertions(+), 12 deletions(-)
> >
> >
> > >
> > > diff --git a/drivers/hid/i2c-hid/i2c-hid-of-elan.c b/drivers/hid/i2c-hid/i2c-hid-of-elan.c
> > > index 029045d9661c..31abab57ad44 100644
> > > --- a/drivers/hid/i2c-hid/i2c-hid-of-elan.c
> > > +++ b/drivers/hid/i2c-hid/i2c-hid-of-elan.c
> > > @@ -18,9 +18,11 @@
> > >  #include "i2c-hid.h"
> > >
> > >  struct elan_i2c_hid_chip_data {
> > > -       unsigned int post_gpio_reset_delay_ms;
> > > +       unsigned int post_gpio_reset_on_delay_ms;
> > > +       unsigned int post_gpio_reset_off_delay_ms;
> > >         unsigned int post_power_delay_ms;
> > >         u16 hid_descriptor_address;
> > > +       const char *main_supply_name;
> > >  };
> > >
> > >  struct i2c_hid_of_elan {
> > > @@ -38,9 +40,11 @@ static int elan_i2c_hid_power_up(struct i2chid_ops *ops)
> > >                 container_of(ops, struct i2c_hid_of_elan, ops);
> > >         int ret;
> > >
> > > -       ret = regulator_enable(ihid_elan->vcc33);
> > > -       if (ret)
> > > -               return ret;
> > > +       if (ihid_elan->vcc33) {
> > > +               ret = regulator_enable(ihid_elan->vcc33);
> > > +               if (ret)
> > > +                       return ret;
> > > +       }
> > >
> > >         ret = regulator_enable(ihid_elan->vccio);
> > >         if (ret) {
> > > @@ -52,8 +56,8 @@ static int elan_i2c_hid_power_up(struct i2chid_ops *ops)
> > >                 msleep(ihid_elan->chip_data->post_power_delay_ms);
> > >
> > >         gpiod_set_value_cansleep(ihid_elan->reset_gpio, 0);
> > > -       if (ihid_elan->chip_data->post_gpio_reset_delay_ms)
> > > -               msleep(ihid_elan->chip_data->post_gpio_reset_delay_ms);
> > > +       if (ihid_elan->chip_data->post_gpio_reset_on_delay_ms)
> > > +               msleep(ihid_elan->chip_data->post_gpio_reset_on_delay_ms);
> > >
> > >         return 0;
> > >  }
> > > @@ -64,8 +68,12 @@ static void elan_i2c_hid_power_down(struct i2chid_ops *ops)
> > >                 container_of(ops, struct i2c_hid_of_elan, ops);
> > >
> > >         gpiod_set_value_cansleep(ihid_elan->reset_gpio, 1);
> > > +       if (ihid_elan->chip_data->post_gpio_reset_off_delay_ms)
> > > +               msleep(ihid_elan->chip_data->post_gpio_reset_off_delay_ms);
> > > +
> > >         regulator_disable(ihid_elan->vccio);
> > > -       regulator_disable(ihid_elan->vcc33);
> > > +       if (ihid_elan->vcc33)
> > > +               regulator_disable(ihid_elan->vcc33);
> > >  }
> > >
> > >  static int i2c_hid_of_elan_probe(struct i2c_client *client)
> > > @@ -89,24 +97,42 @@ static int i2c_hid_of_elan_probe(struct i2c_client *client)
> > >         if (IS_ERR(ihid_elan->vccio))
> > >                 return PTR_ERR(ihid_elan->vccio);
> > >
> > > -       ihid_elan->vcc33 = devm_regulator_get(&client->dev, "vcc33");
> > > -       if (IS_ERR(ihid_elan->vcc33))
> > > -               return PTR_ERR(ihid_elan->vcc33);
> > > -
> > >         ihid_elan->chip_data = device_get_match_data(&client->dev);
> > >
> > > +       if (ihid_elan->chip_data->main_supply_name) {
> > > +               ihid_elan->vcc33 = devm_regulator_get(&client->dev,
> > > +                                                     ihid_elan->chip_data->main_supply_name);
> > > +               if (IS_ERR(ihid_elan->vcc33))
> > > +                       return PTR_ERR(ihid_elan->vcc33);
> > > +       }
> > > +
> > >         return i2c_hid_core_probe(client, &ihid_elan->ops,
> > >                                   ihid_elan->chip_data->hid_descriptor_address, 0);
> > >  }
> > >
> > >  static const struct elan_i2c_hid_chip_data elan_ekth6915_chip_data = {
> > >         .post_power_delay_ms = 1,
> > > -       .post_gpio_reset_delay_ms = 300,
> > > +       .post_gpio_reset_on_delay_ms = 300,
> > > +       .hid_descriptor_address = 0x0001,
> > > +       .main_supply_name = "vcc33",
> > > +};
> > > +
> > > +static const struct elan_i2c_hid_chip_data ilitek_ili9882t_chip_data = {
> > > +       .post_power_delay_ms = 1,
> > > +       .post_gpio_reset_on_delay_ms = 200,
> > > +       .post_gpio_reset_off_delay_ms = 65,
> > >         .hid_descriptor_address = 0x0001,
> > > +       /*
> > > +        * this touchscreen is tightly integrated with the panel and assumes
> > > +        * that the relevant power rails (other than the IO rail) have already
> > > +        * been turned on by the panel driver because we're a panel follower.
> > > +        */
> > > +       .main_supply_name = NULL,
> > >  };
> > >
> > >  static const struct of_device_id elan_i2c_hid_of_match[] = {
> > >         { .compatible = "elan,ekth6915", .data = &elan_ekth6915_chip_data },
> > > +       { .compatible = "ilitek,ili9882t", .data = &ilitek_ili9882t_chip_data },
> >
> > Logically, this patch depends on the panel-follower series that's now
> > landed in drm-misc-next. With your Ack, I'm willing to land these two
> > patches into drm-misc-next too. Other options:
>
> If you are fine with the code, I think it could go with the drm tree
> given that it depends on the panel-follower.
>
> Unless it's too late for you to take 6.6 material (sorry I was off in
> August and just came back).
>
> Acked-By: Benjamin Tissoires <bentiss@xxxxxxxxxx>

Thanks for the Ack, but yeah, it's probably too late for drm-misc.
Hopefully this can go through the normal tree after the next -rc1
then. Thanks!

-Doug




[Index of Archives]     [Linux Media Devel]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Linux Wireless Networking]     [Linux Omap]

  Powered by Linux