RE: [Re-send: PATCH i2c-next 5/6] i2c: mux: mlxcpld: Extend driver to support word address space devices

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

 




> -----Original Message-----
> From: Peter Rosin <peda@xxxxxxxxxx>
> Sent: Thursday, January 14, 2021 11:10 PM
> To: Vadim Pasternak <vadimp@xxxxxxxxxx>; wsa@xxxxxxxxxxxxx; linux-
> i2c@xxxxxxxxxxxxxxx
> Subject: Re: [Re-send: PATCH i2c-next 5/6] i2c: mux: mlxcpld: Extend driver to
> support word address space devices
> 
> Hi!
> 
> On 2021-01-14 19:43, Vadim Pasternak wrote:
> > Hi Peter and Wolfram,
> >
> > Thank you for your comments.
> >
> >> -----Original Message-----
> >> From: Peter Rosin <peda@xxxxxxxxxx>
> >> Sent: Thursday, January 14, 2021 9:49 AM
> >> To: wsa@xxxxxxxxxxxxx; Vadim Pasternak <vadimp@xxxxxxxxxx>; linux-
> >> i2c@xxxxxxxxxxxxxxx
> >> Subject: Re: [Re-send: PATCH i2c-next 5/6] i2c: mux: mlxcpld: Extend
> >> driver to support word address space devices
> >>
> >> Hi!
> >>
> >> On 2021-01-12 11:11, wsa@xxxxxxxxxxxxx wrote:
> >>>
> >>>> Wolfram, is there a better way to get something stable for user
> >>>> space to interact with? Is there maybe a way to do this with aliases or
> something?
> >>>> Setting up an ad-hoc scheme for forcing the adapter IDs feels a bit
> >> outdated.
> >>>
> >>> Yeah, it feels rightfully outdated IMO. Bringing such policy into
> >>> the kernel is frowned upon. I think the proper way is a udev rule to
> >>> act on the newly created I2C adapter. This even could provide a
> >>> really stable symlink for userspace to consume. The above scheme is
> >>> only stable per "block" but inside the block, there is still randomness. Or?
> >>
> >> Right, that makes sense. Thanks! Vadim, is there any reason to not
> >> solve this with udev? Doing that with care could perhaps provide
> >> stable names even if you swap slots?
> >
> > Yes, I can manage it by udev and provide some names like
> > "i2c-lc1-fpga1", which maybe will be more clear for user, then name like "i2c-
> 132".
> >
> > I have another, not user space problem and maybe you can suggest some
> > solution.
> >
> > In line card driver I planned to create I2C infrastructure for the
> > specific line card, like:
> >
> > static int mlxreg_lc_chan[] = {
> > 	0x04, 0x05, 0x06, 0x07, 0x08, 0x10, 0x20, 0x21, 0x22, 0x23, 0x40, 0x41,
> > 	0x42, 0x43, 0x44, 0x45, 0x46, 0x47, 0x48, 0x49, 0x4a, 0x4b, 0x4c, 0x4d,
> > 	0x4e, 0x4f
> > };
> >
> > static struct mlxcpld_mux_plat_data mlxreg_lc_mux_data[] = {
> > 	{
> > 		.chan_ids = mlxreg_lc_chan,
> > 		.num_adaps = ARRAY_SIZE(mlxreg_lc_chan),
> > 		.sel_reg_addr = MLXREG_LC_CHANNEL_I2C_REG,
> > 		.reg_size = 2,
> > 	},
> > };
> >
> > 	mlxreg_lc->mux = platform_device_register_resndata(dev, "i2c-mux-
> mlxcpld", parent_nr,
> > 							   NULL, 0,
> &mlxreg_lc_mux_data,
> >
> sizeof(mlxreg_lc_mux_data));
> >
> > And after this infrastructure is ready - to attach from this drive
> > line card devices from 'i2c_board_info', like:
> >
> > static struct i2c_board_info mlxreg_lc_main_pwr_devices[] = {
> > 	{
> > 		I2C_BOARD_INFO("mp2975", 0x62),
> > 	},
> > 	{
> > 		I2C_BOARD_INFO("mp2975", 0x64),
> > 	},
> > 	{
> > 		I2C_BOARD_INFO("max11603", 0x6d),
> > 	},
> > 	{
> > 		I2C_BOARD_INFO("lm25066", 0x15),
> > 	},
> > };
> >
> > static struct mlxreg_hotplug_device mlxreg_lc_main_pwr_brdinfo[] = {
> > 	{
> > 		.brdinfo = &mlxreg_lc_main_pwr_devices[0],
> > 		.nr = 4,
> > 	},
> > 	{
> > 		.brdinfo = &mlxreg_lc_main_pwr_devices[1],
> > 		.nr = 4,
> > 	},
> > 	{
> > 		.brdinfo = &mlxreg_lc_main_pwr_devices[2],
> > 		.nr = 5,
> > 	},
> > 	{
> > 		.brdinfo = &mlxreg_lc_main_pwr_devices[3],
> > 		.nr = 6,
> > 	},
> > };
> >
> > Where the above 'nr's are from 'mlxreg_lc_chan'.
> >
> > And then create with i2c_new_client_device() all the above devices
> > from workqueue, which will be running until all the 'mlxreg_lc_chan'
> > related adapters are created.
> > With forcing base nr, I know the number of last nr, which should be
> > created by "i2c-mux-mlxcpld".
> >
> > Without it I'll need some ability to find with nrs have been created
> > by "i2c-mux-mlxcpld".
> > Do you have any suggestions for that?
> >
> > I understand that I can also do it through udev, but I'd prefer to
> > create all on-board (line card) devices from the kernel, if possible.
> 
> You could add a callback function to struct mlxcpld_mux_plat_data, and have
> the driver call you back with the mapping so that you know what adapter ID
> you got for each platform data (or channel if needed) you instantiate.
> 
> Would that do it?

Great!!!
I will do it in this way.

Thank you very much for help.

I'll add it to v2 patchset.

Cheers,
Vadim.

> 
> Cheers,
> Peter




[Index of Archives]     [Linux GPIO]     [Linux SPI]     [Linux Hardward Monitoring]     [LM Sensors]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux