More flexible USB setup flow

[gsora]

This PR adds a way for users to handle custom USB descriptor
requests by adding a new Device struct field called DescriptorSetup,
just like the Setup field.

A new field called Conditional has been added to the Device type.

This field let the user decide whether a SetupData request should be handled
by usb.doSetup() or by other logic defined in Conditiona function logic.

Both functions are optional, and their presence doesn't impact the logic
currently implemented in Tamago's USB stack.

[abarisani]

I think the SetupConditionalFunc naming, and description, is a little confusing in relation to the existing code base.

We already do advertise Device.Setup as the optional class-specific setup handler so I think we need to clearly distinguish the role of the two.

I think we should simply have Device.GetDescriptor for optional class-specific descriptors, to reflect the same naming scheme.

By doing so would there really be a need to override the existing functions? Rather than keeping custom ones as default switch targets in case everything else fails?

If a total override is needed wouldn't a simple boolean on device be better? We could export getDescriptor and doSetup as Descriptor and Setup to facilitate the override with a single boolean for both (so that the app can then re-use them). The boolean could be named ExternalSetup or something.

[gsora]

On second thought maybe my naming scheme isn't the the best - a solution like the one you proposed surely looks cleaner.

A thing like:

// SetupFunction represents the function to process class-specific setup
// requests.
type SetupFunction func(setup *SetupData) (in []byte, err error)

// Device is a collection of USB device descriptors and host driven settings
// to represent a USB device.
type Device struct {
	Descriptor     *DeviceDescriptor
	Qualifier      *DeviceQualifierDescriptor
	Configurations []*ConfigurationDescriptor
	Strings        [][]byte

	// Host requested settings
	ConfigurationValue uint8
	AlternateSetting   uint8

	// Optional class-specific setup handler
	Setup SetupFunction
	CustomDescriptors SetupFunction
        ExternalSetup bool
}

where Setup() is used for custom setup.Request values and Descriptor() for custom descriptor types.

device.ExternalSetup might be a good idea for those who want to implement very exotic USB devices, but I'm not sure it would be needed with Descriptor() and Setup().

If you're okay with this I can implement those changes on this PR.

[abarisani]

Given that I favor a minimal approach, for the sake of clarity, I propose the following (which goes somewhat in line to what you proposed originally but makes it even simpler):

• we keep Setup SetupFunction as it is but with added boolean as return
• we move the check of a SetupFunction attached to the device before the switch so that it operates before all standard cases

I hope this can speak for itself:

func (hw *USB) doSetup(dev *Device, setup *SetupData) (err error) {
        if setup == nil {
                return
        }

        if dev.Setup != nil { 
                in, done, err := dev.Setup(setup)

                if err != nil {
                        hw.stall(0, IN)
                } else if len(in) != 0 {
                        err = hw.tx(0, false, in)
                } else {
                        err = hw.ack(0)
                }

                if done {
                        return
                }
        }

...

       default:
                hw.stall(0, IN)
                return fmt.Errorf("unsupported request code: %#x", setup.Request)
        }

Does this make sense?

This way I think we allow for maximum freedom and least code changes (we don't add functions or booleans and we very slightly change the behaviour of Device.Setup.

[gsora]

Makes sense.

we don't add functions or booleans

What about CustomDescriptors()?

Your solution could work for that too.

[abarisani]

Makes sense.

we don't add functions or booleans

What about CustomDescriptors()?

Your solution could work for that too.

There is no need for CustomDescriptors() with my solution isn't there? doSetup is parent of that processing so you wouldn't need more to process custom descriptors wouldn't you?