Re: [PATCH v3 2/3] drivers: hv: vmbus: add test attributes to debugfs

[Branden Bonaby]

> > +What:           /sys/kernel/debug/hyperv/<UUID>/fuzz_test_state
> > +Date:           August 2019
> > +KernelVersion:  5.3
> > +Contact:        Branden Bonaby <brandonbonaby94@xxxxxxxxx>
> > +Description:    Fuzz testing status of a vmbus device, whether its in an ON
> > +                state or a OFF state
> 
> Document what values are actually returned?  
> 
> > +Users:          Debugging tools
> > +
> > +What:           /sys/kernel/debug/hyperv/<UUID>/delay/fuzz_test_buffer_interrupt_delay
> > +Date:           August 2019
> > +KernelVersion:  5.3
> > +Contact:        Branden Bonaby <brandonbonaby94@xxxxxxxxx>
> > +Description:    Fuzz testing buffer delay value between 0 - 1000
> 
> It would be helpful to document the units -- I think this is 0 to 1000
> microseconds.

you're right, that makes sense I'll add that information in. Also 
to confirm, it is microseconds like you said.

> > +static int hv_debugfs_delay_set(void *data, u64 val)
> > +{
> > +	if (val >= 1 && val <= 1000)
> > +		*(u32 *)data = val;
> > +	/*Best to not use else statement here since we want
> > +	 * the delay to remain the same if val > 1000
> > +	 */
> 
> The standard multi-line comment style would be:
> 
> 	/*
> 	 * Best to not use else statement here since we want
> 	 * the delay to remain the same if val > 1000
> 	 */
>

will change

> > +	else if (val <= 0)
> > +		*(u32 *)data = 0;
> 
> You could consider returning an error for an invalid
> value (< 0, or > 1000).
> 

its subtle but it does make sense and shows anyone
reading that the only acceptable values in the 
function are 0 <= 1000 at a glance. I'll add
that in.