Aspeed vhub configuration

Aspeed vhub configuration

[Benjamin Herrenschmidt]

Hi folks !

The Aspeed USB gadget "vhub" implements a hub emulation with a number
of UDCs representing the hub slots. It's working ok now and has been
upstream for a bit, however, one thing that's been annoying to some
users is that I've just hard coded the hub's device descriptor. IE, the
vendor/product ID, strings etc...

Various BMC SW stack vendors want to customize that, also possibly the
number of ports etc...

I originally thought about configfs but after more thoughts I don't
think it's really a good fit. The vhub is a fixed thing. When you have
the HW, you have that hub, it's not like you can create different
things, and populate differently.

That leaves me with two approaches, that aren't mutually exclusive, but
I'd like to run them past the folks here before I start coding:

 - The defaults, currently hard coded, could be replaced with Kconfig
options.

 - The device-tree node could contain optional override of those
defaults, allowing a vendor to customize the hub for a given board.
It's not per-se a HW description, but the device-tree is also fairly
commonly used for HW configuration, even if some people disagree with
me on that one (hint: they are wrong :-)

 - I could add sysfs properties underneath the vhub device instance to
customize it. This would also allow userspace to control whether the
hub is "connected" to the host or not, which could be useful, some
systems don't want it to always be there. Today there's no choice.

Any other option ? If somebody says netlink I will scream :)

Cheers,
Ben.

Re: Aspeed vhub configuration

[Greg KH]

On Tue, Jul 02, 2019 at 09:33:11PM +1000, Benjamin Herrenschmidt wrote:
> Hi folks !
> 
> The Aspeed USB gadget "vhub" implements a hub emulation with a number
> of UDCs representing the hub slots. It's working ok now and has been
> upstream for a bit, however, one thing that's been annoying to some
> users is that I've just hard coded the hub's device descriptor. IE, the
> vendor/product ID, strings etc...
> 
> Various BMC SW stack vendors want to customize that, also possibly the
> number of ports etc...
> 
> I originally thought about configfs but after more thoughts I don't
> think it's really a good fit. The vhub is a fixed thing. When you have
> the HW, you have that hub, it's not like you can create different
> things, and populate differently.
> 
> That leaves me with two approaches, that aren't mutually exclusive, but
> I'd like to run them past the folks here before I start coding:
> 
>  - The defaults, currently hard coded, could be replaced with Kconfig
> options.
> 
>  - The device-tree node could contain optional override of those
> defaults, allowing a vendor to customize the hub for a given board.
> It's not per-se a HW description, but the device-tree is also fairly
> commonly used for HW configuration, even if some people disagree with
> me on that one (hint: they are wrong :-)
> 
>  - I could add sysfs properties underneath the vhub device instance to
> customize it. This would also allow userspace to control whether the
> hub is "connected" to the host or not, which could be useful, some
> systems don't want it to always be there. Today there's no choice.
> 
> Any other option ? If somebody says netlink I will scream :)

DT seems like the logical choice, I'll not object to that.

thanks,

greg k-h

Re: Aspeed vhub configuration

[Benjamin Herrenschmidt]

On Tue, 2019-07-02 at 14:24 +0200, Greg KH wrote:
> 
> > That leaves me with two approaches, that aren't mutually exclusive, but
> > I'd like to run them past the folks here before I start coding:
> > 
> >  - The defaults, currently hard coded, could be replaced with Kconfig
> > options.
> > 
> >  - The device-tree node could contain optional override of those
> > defaults, allowing a vendor to customize the hub for a given board.
> > It's not per-se a HW description, but the device-tree is also fairly
> > commonly used for HW configuration, even if some people disagree with
> > me on that one (hint: they are wrong :-)
> > 
> >  - I could add sysfs properties underneath the vhub device instance to
> > customize it. This would also allow userspace to control whether the
> > hub is "connected" to the host or not, which could be useful, some
> > systems don't want it to always be there. Today there's no choice.
> > 
> > Any other option ? If somebody says netlink I will scream :)
> 
> DT seems like the logical choice, I'll not object to that.

DT for defaults sounds good yes. I'm still toying with also having
sysfs properties. There have been people wanting to connect or
disconnect the hub from the host programatically, a property in the
device node would be best for that. Being able to override the serial
number as well. The DT isn't a great place for that.

Cheers,
Ben.

Re: Aspeed vhub configuration

[Felipe Balbi]

Hi,

Benjamin Herrenschmidt <benh@kernel.crashing.org> writes:

> On Tue, 2019-07-02 at 14:24 +0200, Greg KH wrote:
>> 
>> > That leaves me with two approaches, that aren't mutually exclusive, but
>> > I'd like to run them past the folks here before I start coding:
>> > 
>> >  - The defaults, currently hard coded, could be replaced with Kconfig
>> > options.
>> > 
>> >  - The device-tree node could contain optional override of those
>> > defaults, allowing a vendor to customize the hub for a given board.
>> > It's not per-se a HW description, but the device-tree is also fairly
>> > commonly used for HW configuration, even if some people disagree with
>> > me on that one (hint: they are wrong :-)
>> > 
>> >  - I could add sysfs properties underneath the vhub device instance to
>> > customize it. This would also allow userspace to control whether the
>> > hub is "connected" to the host or not, which could be useful, some
>> > systems don't want it to always be there. Today there's no choice.
>> > 
>> > Any other option ? If somebody says netlink I will scream :)
>> 
>> DT seems like the logical choice, I'll not object to that.
>
> DT for defaults sounds good yes. I'm still toying with also having
> sysfs properties. There have been people wanting to connect or
> disconnect the hub from the host programatically, a property in the
> device node would be best for that. Being able to override the serial
> number as well. The DT isn't a great place for that.

We already have an interface for disconnecting from the host
programatically by disconnecting data pullup.

static ssize_t soft_connect_store(struct device *dev,
		struct device_attribute *attr, const char *buf, size_t n)
{
	struct usb_udc		*udc = container_of(dev, struct usb_udc, dev);

	if (!udc->driver) {
		dev_err(dev, "soft-connect without a gadget driver\n");
		return -EOPNOTSUPP;
	}

	if (sysfs_streq(buf, "connect")) {
		usb_gadget_udc_start(udc);
		usb_gadget_connect(udc->gadget);
	} else if (sysfs_streq(buf, "disconnect")) {
		usb_gadget_disconnect(udc->gadget);
		usb_gadget_udc_stop(udc);
	} else {
		dev_err(dev, "unsupported command '%s'\n", buf);
		return -EINVAL;
	}

	return n;
}
static DEVICE_ATTR_WO(soft_connect);

part of udc/core.c

-- 
balbi

Re: Aspeed vhub configuration

[Benjamin Herrenschmidt]

On Tue, 2019-07-02 at 15:56 +0300, Felipe Balbi wrote:
> We already have an interface for disconnecting from the host
> programatically by disconnecting data pullup.
> 
> static ssize_t soft_connect_store(struct device *dev,
>                 struct device_attribute *attr, const char *buf,
> size_t n)

 .../...

I could reproduce the same interface for the vhub, at least it would
provide consistency...

The vhub itself is not an UDC device. It provides UDC devices that are
the hub "ports".

Cheers,
Ben.

Re: Aspeed vhub configuration

[Felipe Balbi]

Hi,

Benjamin Herrenschmidt <benh@kernel.crashing.org> writes:

> On Tue, 2019-07-02 at 15:56 +0300, Felipe Balbi wrote:
>> We already have an interface for disconnecting from the host
>> programatically by disconnecting data pullup.
>> 
>> static ssize_t soft_connect_store(struct device *dev,
>>                 struct device_attribute *attr, const char *buf,
>> size_t n)
>
>  .../...
>
> I could reproduce the same interface for the vhub, at least it would
> provide consistency...
>
> The vhub itself is not an UDC device. It provides UDC devices that are
> the hub "ports".

Right, if you have your UDC devices and you implement ->udc_start(),
->udc_stop() and ->pullup(), you *already* have this interface for you.

-- 
balbi

Re: Aspeed vhub configuration

[Benjamin Herrenschmidt]

On Wed, 2019-07-03 at 09:24 +0300, Felipe Balbi wrote:
> Hi,
> 
> Benjamin Herrenschmidt <benh@kernel.crashing.org> writes:
> 
> > On Tue, 2019-07-02 at 15:56 +0300, Felipe Balbi wrote:
> > > We already have an interface for disconnecting from the host
> > > programatically by disconnecting data pullup.
> > > 
> > > static ssize_t soft_connect_store(struct device *dev,
> > >                 struct device_attribute *attr, const char *buf,
> > > size_t n)
> > 
> >  .../...
> > 
> > I could reproduce the same interface for the vhub, at least it
> > would
> > provide consistency...
> > 
> > The vhub itself is not an UDC device. It provides UDC devices that
> > are
> > the hub "ports".
> 
> Right, if you have your UDC devices and you implement ->udc_start(),
> ->udc_stop() and ->pullup(), you *already* have this interface for
> you.

I have that interface for the hub ports. What I want is a way for
userspace to connect/disconnect the hub itself from the host though.

Cheers,
Ben.