On Wed, Dec 3, 2014 at 12:16 PM, <jilaiw@xxxxxxxxxxxxxx> wrote: >>>> + enum hdmi_hdcp_state hdcp_state; >>>> + struct mutex state_mutex; >>>> + struct delayed_work hdcp_reauth_work; >>>> + struct delayed_work hdcp_auth_part1_1_work; >>>> + struct delayed_work hdcp_auth_part1_2_work; >>>> + struct work_struct hdcp_auth_part1_3_work; >>>> + struct delayed_work hdcp_auth_part2_1_work; >>>> + struct delayed_work hdcp_auth_part2_2_work; >>>> + struct delayed_work hdcp_auth_part2_3_work; >>>> + struct delayed_work hdcp_auth_part2_4_work; >>>> + struct work_struct hdcp_auth_prepare_work; >>> >>> You shouldn't use "work" as a way to express states in your state >>> machine. >>> Better have 1 auth work function that does all these steps, probably >>> having >>> them split in functions just like you do now. >>> >>> That way you can have 1 function running the pass of authentication, >>> starting >>> by checking if you're reauthing or not then processing each step one by >>> one, >>> sleeping inbetween them. You can have the functions return -EAGAIN to >>> indicate >>> that you need to retry the current operation and so on. >>> >>> This would split the state machine from the state executioners and >>> simplify >>> your code. >> >> As a side note (disclaimer, I'm not an hdcp expert), but I wonder if >> eventually some of that should be extracted out into some helpers in >> drm core. I guess that is something we'll figure out when a 2nd >> driver gains upstream HDCP support. One big work w/ msleep()'s does >> sound like it would be easier to understand (and therefore easier to >> refactor out into helpers). >> >> [snip] >> > > The reason that I break the partI/PartII work into these small works > because I want to avoid to use msleep in work. > Otherwise cancel a HDCP work may cause long delay up to several seconds. hmm, yeah, several seconds is long enough for user to plug/unplug several displays ;-) But I guess a wait_event_timeout() plus a wq that is signalled on hpd (or whenever we need to cancel) instead of msleep might do the trick? BR, -R -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
