Re: [PATCH v2 2/8] usb: typec: fusb302: Refactor / simplify tcpm_set_cc()

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

 



On Thu, Mar 07, 2019 at 10:12:59AM -0800, Guenter Roeck wrote:
> On Thu, Mar 07, 2019 at 05:36:01PM +0100, Hans de Goede wrote:
> > After commit ea3b4d5523bc ("usb: typec: fusb302: Resolve fixed power role
> > contract setup"), tcpm_set_cc always calls fusb302_set_toggling.
> > 
> > Before this refactor tcpm_set_cc does the following:
> > 
> > 1) fusb302_set_toggling(TOGGLING_MODE_OFF),
> >    this sets both FUSB_REG_MASK_BC_LVL and FUSB_REG_MASK_COMP_CHNG.
> > 2) fusb302_set_cc_pull(...).
> > 3) "reset cc status".
> > 4) if pull-up fusb302_set_src_current(...).
> > 5) if pull-up or pull-down enable bc-lvl resp comp-chng irq.
> > 6) fusb302_set_toggling(new-toggling-mode), which again
> >    sets both FUSB_REG_MASK_BC_LVL and FUSB_REG_MASK_COMP_CHNG disabling
> >    the just enabled irq. fusb302_set_toggling is skipped when the new
> >    toggling mode is TOGGLING_MODE_OFF because this is already done in 1,
> >    note in this case 5) is a no-op.
> > 
> > When we are toggling the bits set by fusb302_set_cc_pull will be ignored
> > until we turn toggling off, so we can safely move the fusb302_set_cc_pull
> > call to before setting TOGGLING_MODE_OFF.
> > 
> > Either we are not toggling yet, or the src-current has already been set,
> > so we can also safely set the src-current earlier, allowing us to do the
> > fusb302_set_toggling(TOGGLING_MODE_OFF) call at the same time as we
> > set the other toggling modes. Also setting the src-current is a no-op
> > when not enabling pull-ups, so we can drop the if.
> > 
> > And since the second fusb302_set_toggling undoes the effects of step 5,
> > we can skip step 5, the bc-lvl resp comp-chng irq wil be enabled by
> > fusb302_handle_togdone_snk resp. fusb302_handle_togdone_src when toggling
> > is done.
> > 
> > Together this allows us to simplify things to:
> > 
> > 1) fusb302_set_cc_pull(...)
> > 2) "reset cc status"
> > 3) fusb302_set_src_current(...)
> > 4) fusb302_set_toggling(new-toggling-mode)
> > 
> > This commit does this, leading to a nice cleanup.
> > 
> > Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
> 
> I think I understand the logic, but I think it would be really beneficial
> if this (and the rest of the series) can be tested independently (ie
> by someone with hardware).
> 
> Reviewed-by: Guenter Roeck <linux@xxxxxxxxxxxx>

I'll test this series with my GPD win board today.

thanks,

-- 
heikki



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux