RE: [PATCH led-next 1/1] leds: mlxreg: Allow multi-instantiation of same name LED for modular systems

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

 




> -----Original Message-----
> From: Pavel Machek <pavel@xxxxxx>
> Sent: Wednesday, October 21, 2020 11:34 AM
> To: Vadim Pasternak <vadimp@xxxxxxxxxx>
> Cc: Marek Behun <marek.behun@xxxxxx>; jacek.anaszewski@xxxxxxxxx; linux-
> leds@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH led-next 1/1] leds: mlxreg: Allow multi-instantiation of
> same name LED for modular systems
> 
> Hi!
> 
> > > > But why you consider it as function enumerator?
> > > > For example card48, card56 are two different devices of same type.
> > > > Both have 'status' LED.
> > >
> > > OK this is a fair point.
> > >
> > > I was thinking such because in my mind I had this idea that for an
> > > ethernet switch with interfaces lan0 - lan4 it would make sense to
> > > use the LED_FUNCTION_LAN function with function enumerator. But
> > > thinking about this now again makes me wonder if instead the lan0 -
> > > lan4 should be devicenames instead, since normally they are network
> interface names.
> > >
> > > Vadim, the reason why Pavel and I think that mlxreg (or mlxregN) is
> > > not valid devicename part (although mlxreg has to stay since many
> > > users already depend on it, as you say), is that the mlxreg name is
> > > not exposed anywhere else in Linux from userspace point of view.
> > >
> > > Devicename eth0 is okay, because it is network interface name.
> > > Devicename sda would be okay, because everyone knows it is a block
> > > device and you can access it via /dev/sda.
> > > Devicename hci0 would be okay because it is bluetooth interface
> > > accessible via hcitool.
> > > Devicenames mtd0, kbd0, mouse0 would be okay, I think.
> > >
> > > But mlxreg is not accessible via anything else in the system. Unless
> > > your systems also have something like /dev/mlxreg, that is.
> > >
> > > Do the LEDs on these cards only indicate status of the cards
> > > themselves as a whole? Or are there LEDs on these cards dedicated to
> > > their peripherals? For example if there is an ethernet port with
> > > LEDs on one of these cards, the devicename part for these LEDs
> > > should be of the device of that ethernet port, not mlxreg...
> >
> > Hi Marek,
> >
> > Each line card must have 'status' LED, indicating status of line card itself.
> > User can set non-green in case some there are some alarms on different
> > devices, equipped on this line card. It can be set blink during line card
> initialization.
> >
> > Line card could be equipped with UID LED. User can set this LED in
> > order to find physical location of line card. Sometimes it's hard to
> > see the sticker on chassis.
> >
> > Line card also equipped with per port LED, but those LEDs are handled by
> FW.
> >
> > So, the device in this case is 'line card'.
> >
> > In my previous reply I suggest name 'fru' stands for the filed replaceable unit.
> > This is not something, that is exposed in '/dev', but it describes any
> > replaceable unit within the system.
> 
> So.. you'd use the LED to locate right PCI card, or the LED would indicate that
> whole card is failing, etc...?
> 
> Could we use pci00:1b.0 as the device name? (same as lspci). Probably replace
> : with _...
> 

Hi Pavel,

Yes, STATU and UID LED indicates whole line card status/location.

Some line cards could be connected through PCIe, but not all.
>From chassis management perspective they are connected by I2C.
And CPLD register map is accessed through I2C.

Following your suggestion it could be i2c-{n} as device name (pdev->id is I2C bus):

 		if (priv->pdev->id > 0)
 			sprintf(led_data->led_cdev_name, "%s%d:%s", "i2c-",
 				priv->pdev->id, data->label);
 		else
 			sprintf(led_data->led_cdev_name, "%s:%s", "mlxreg",
 				data->label);

Or bus name could be passed through the 'identity' filed:
		if (priv->pdev->id > 0)
			sprintf(led_data->led_cdev_name, "%s%d:%s", led_pdata->identity,
				priv->pdev->id, data->label);
		else
			sprintf(led_data->led_cdev_name, "%s:%s", "mlxreg",
				data->label);

Thanks,
Vadim.

> Best regards,
> 								Pavel
> --
> http://www.livejournal.com/~pavelmachek




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux