Support for per-camera (or per EEPROM) DTB overlays #53

eh-steve · 2023-05-18T01:39:53Z

This PR adds 4 new features:

A new I2cSlaveDeviceTreePath protocol to allow the lookup of the device tree path of any I2cIo device using its GUID and globally unique device index (+ also fixed bugs with non-unique device indices)
PCA9547 i2cmux support via the EFI_I2C_BUS_CONFIGURATION_MANAGEMENT_PROTOCOL. This allows the bootloader to traverse i2cmux sub-nodes in the device tree, and to switch between mux channels in order to communicate with these I2C slaves. While currently only pca9547 compatible muxes are supported (and must be on the same bus), it should be fairly simple to add others (e.g. i2c-mux-gpio compatible or pca9546/pca9548 compatible switch) with a small refactor.
A new DTB overlay field inside board_config - eeprom-dt-paths. This can be used in tandem with ids to specify the exact EEPROM device which must match the IDs in order to apply a given overlay - this allows multiple camera models/manufacturers to be plugged in and supported at the same time via selectively applied overlays, and also to only selectively enable cameras/devices which are plugged in
Support for Framos camera EEPROMs (skip the CRC check same as LPRD).

Overall, this PR enables the use of a 6 camera/12 channel base device tree with all devices disabled by default like:


/ {
	i2c@3180000 {
		pca9547@70 {
			compatible = "nxp,pca9547";
			reg=<0x70>;
			cam_mux0: i2c@0 {
				reg = <0>;
				eeprom@55 {
					compatible = "atmel,24c02";
					reg = <0x55>;
				};
				eeprom@56 {
					compatible = "atmel,24c02";
					reg = <0x56>;
				};
				imx462_a@1a {
					status = "disabled";
					... etc.
				}
			}
			... etc.
			cam_mux5: i2c@5 {
				reg = <5>;
				eeprom@55 {
					compatible = "atmel,24c02";
					reg = <0x55>;
				};
				eeprom@56 {
					compatible = "atmel,24c02";
					reg = <0x56>;
				};
				imx462_f@1a {
					status = "disabled";
					...
				}
			}
		}
	}

	tegra-camera-platform {
		modules {
                        // Disabling all modules unless the EEPROM is probed and the relevant overlay is applied by UEFI bootloader prevents nvargus-daemon from erroring due to missing devices
			module0 {
				status = "disabled";
				... etc.
			};
			...
			module5 {
				status = "disabled";
				... etc.
			};

	}
	tegra-capture-vi {
		num-channels = <6>;
		ports {
			 port@0 {
				reg = <0>;
				endpoint {
					status = "disabled";
					... etc.
				}
			}
			... etc.
			 port@5 {
				reg = <0>;
				endpoint {
					status = "disabled";
					... etc.
				}
			}
		}
	}
}

Then by using 6 separate DTB overlays (one for each camera) like:

	overlay-name = "Camera 0";
	fragment@0 {
		target-path = "/i2c@3180000/pca9547@70/i2c@0/imx462_a@1a";
		board_config {
			ids = "framos-imx462-0";
			eeprom-dt-paths = "/i2c@3180000/pca9547@70/i2c@0/eeprom@55", "/i2c@3180000/pca9547@70/i2c@0/eeprom@56";
			sw-modules = "kernel";
		};
		__overlay__ {
			status = "okay";
		};
	};

	fragment@1 {
		target-path = "/tegra-camera-platform/modules/module0";
		board_config {
			ids = "framos-imx462-0";
			eeprom-dt-paths = "/i2c@3180000/pca9547@70/i2c@0/eeprom@55", "/i2c@3180000/pca9547@70/i2c@0/eeprom@56";
			sw-modules = "kernel";
		};
		__overlay__ {
			status = "okay";
		};
	};

	fragment@2 {
		target-path = "/tegra-capture-vi/ports/port@0/endpoint";
		board_config {
			ids = "framos-imx462-0";
			eeprom-dt-paths = "/i2c@3180000/pca9547@70/i2c@0/eeprom@55", "/i2c@3180000/pca9547@70/i2c@0/eeprom@56";
			sw-modules = "kernel";
		};
		__overlay__ {
			status = "okay";
		};
	};

The bootloader can selectively enable overlays/cameras whose EEPROMs match both the ids and eeprom-dt-paths, without triggering overlays for devices which aren't present on a particular port.

ashishsingha · 2023-05-18T04:01:56Z

Hello,

I am not sure I am able to understand the goal of what you are trying to do. NV UEFI reads all EEPROMs based on information in eeprom-manager node in the DTB. All EEPROMs read from this are then used to apply the overlays to kernel DTB.

Thanks
Ashish

eh-steve · 2023-05-18T07:58:57Z

Hi,

I am not sure I am able to understand the goal of what you are trying to do. NV UEFI reads all EEPROMs based on information in eeprom-manager node in the DTB. All EEPROMs read from this are then used to apply the overlays to kernel DTB.

The current behaviour of NV UEFI is that:

if any EEPROM product ID from any I2C bus matches any of the ids listed in board_config, the overlay is triggered. Effectively all EEPROMs add to a global list of Board IDs, regardless of which device they actually represent. This means a single camera plugged into a single port would make the whole board be treated as matching that camera.
Any atmel compatible device on any I2C bus will be read, not just those listed in /eeprom-manager (This is fine/good)
EEPROMs behind an I2C mux are not readable by UEFI, even if correctly described in the device tree (whether the mux specified is in /eeprom-manager or directly under the relevant bus node).

This behaviour is a problem as it:

Prevents plugging different camera models into different ports and have them work without re-flashing the whole device (any CSI camera port change requires a reflash, instead of supporting various models automatically, and having the interposer board partly populated)
Causes the kernel to create devnodes for devices that don't exist, (causing preventable error logs in nvargus-daemon)
Multiple cameras are almost always behind an I2C mux on most interposer board hardware (including the reference e3333), so these EEPROMs don't get read.

This PR adds support for a new, more controlled overlay behaviour via the new (optional) eeprom-dt-paths board_config property, where an overlay can be triggered based on a match of the ID in the EEPROM and a match of the exact I2C device tree path the EEPROM product ID came from. (Old behaviour with existing board_configs should be unchanged). This means that the presence of a single camera no longer adds this Product ID to the global list of board IDs, but that you can selectively apply an overlay only when a specific ID was read from a specific device tree path (i.e. individual CSI cameras can be enumerated during boot and the kernel never creates devnodes for missing devices).

So for this board_config:

board_config {
	ids = "framos-imx462-0";
	eeprom-dt-paths = "/i2c@3180000/pca9547@70/i2c@0/eeprom@55", "/i2c@3180000/pca9547@70/i2c@0/eeprom@56";
	sw-modules = "kernel";
};

The overlay will only be applied if an EEPROM is read from one of those 2 DT paths, and that EEPROM Product ID also matches framos-imx462-0.

Let me know if anything else is unclear

eh-steve · 2023-05-18T14:17:09Z

(This also fixes a prior bug where devices with no phandle in the device tree will end up with duplicate device indexes of 0. Now the combination of GUID + device index is guaranteed to uniquely identify a device)

ashishsingha · 2023-05-18T16:37:44Z

Hello,

Let's go over the issues one by one to be able to handle them better. My comment will be purely from the perspective of NV providing UEFI for its reference platform and what we want to support there and via what mechanism.

Support for Framos camera EEPROMs: I have not been told by anyone from our internal product teams about this camera EEPROM hence its support is missing in our UEFI. We highly recommend users follow the EEPROM layout as described by NV and have a valid CRC. Even for the one-off case you mentioned for LPRD, that is for backward compatibility reasons only. If a user has an EEPROM where they want to skip the CRC check, we suggest users make that change in NV UEFI locally as NV does not recommend doing that. What we would do to make this easy is, give a PCD which a customer can flip if someone does not want to enforce a CRC check.
Tegra I2C Controller Unique ID: I see that as an issue and would work on a way to fix that. We would probably go towards the approach of not initializing a controller that does not have a phandle as that means there is nobody talking to that I2C.
Support for PCA9547: There is no NV reference platform that has this particular I2C bus multiplexer so you do not see this supported right now. However, if you can provide a commit specifically for this and you have verified this, I would absorb that change happily.
Enhancement To Overlays Application: UEFI provides support for applying overlays so that a user can potentially keep the same FW set and still be able to boot multiple board variants by applying overlays to UEFI and kernel DTB to reflect state of the platform more correctly.
4.1 Overlays Application To UEFI DTB: Bootloaders running before UEFI read the base board and module EEPROMs and provide the data to UEFI. UEFI then in its SEC phase applies the overlays to its own DTB based on the ID read from board and module EEPROMs.
4.2 Overlays Application To Kernel DTB: UEFI processes the EEPROM-manager node in the DTB that has information about all EEPROMs that are supposed to be read and their information needs to be used. Any EEPROM listed over there is read (if found) and the IDs read from here are then used to apply overlays to kernel DTB as well as passed to the kernel via a property in /chosen node in the DTB. What you are suggesting, is basically a different way of how we find out about EEPROMs where every EEPROM is listed as a child device under the i2c bus itself. I do see the use case where you may have the same EEPROM available on multiple I2C buses but want to apply overlay based on EEPROM data read from a specific EEPROM on a specific I2C bus. I would talk to our internal platform and product team to see what they think about this. As I mentioned earlier, NV UEFI is the reference code for the reference boards we provide and users are expected to make changes for their specific use case themselves and maintain them too. I cannot promise at this point that we would be able to provide a solution to this specific case as that would mean we need to test and maintain it despite not having a platform to do so.

Thanks
Ashish

eh-steve · 2023-05-18T18:20:18Z

Thanks for coming back to me.
For 1), that sounds like a nice option. I'll also speak to Framos and see if they can add the last CRC8 byte as part of their manufacturing process to avoid it (although in the short term I can write the checksum byte myself). It would be even nicer if the CRC check could be skipped based on a device tree property, as this would mean users don't need to compile a custom bootloader.

2. We would probably go towards the approach of not initializing a controller that does not have a phandle as that means there is nobody talking to that I2C.

I don't think this is a safe assumption at all - it's quite standard for EEPROM nodes not to be referenced by other nodes - that doesn't mean nothing is talking to those devices. The existence of a phandle doesn't tell you anything about how a driver might interpret a device tree node, especially those with compatible props set. As a case in point, the current implementation doesn't even expect phandles to exist on the /eeprom-manager nodes. The current (master) code is buggy, and needs some sort of solution for guaranteeing globally unique device indices (I've just implemented it via bus ID + device number).

Support for PCA9547: There is no NV reference platform that has this particular I2C bus multiplexer so you do not see this supported right now. However, if you can provide a commit specifically for this and you have verified this, I would absorb that change happily.

The reference e3333 interposer uses a very similar TCA9546A switch - I'd be happy to add support for that as well, as a reference implementation, and keep my support for the PCA9547 (which I've verified locally on my custom board). Let me know if you'd like me to pick this task up.

What you are suggesting, is basically a different way of how we find out about EEPROMs where every EEPROM is listed as a child device under the i2c bus itself

To be clear, NV UEFI already supports adding atmel compatible devices defined directly under the i2c bus node instead of eeprom-manager (I didn't need to add that). What I added was the ability to resolve and communicate with nested devices behind an i2cmux/switch, which was previously missing. I also added the ability to know which device path an EEPROM read came from.

I appreciate you might need to check in with the product team to decide whether you want to add this feature, but I think it would be very useful, even for your reference e3333 interposer, not just on custom carriers, (and you should be able to test/verify with it once I add the TCA9546A support).

It would allow CSI cameras to behave more like Plug and Play devices via a single, base device tree containing all supported variants (but disabled), with 7 different overlays, one targeting the interposer EEPROM, and one for each of the 6 camera EEPROMs. Then when an interposer board is partly populated with a subset of cameras, you don't need to reflash the Tegra for that configuration, it gets automatically detected and the correct device tree applied automatically.

ashishsingha · 2023-05-18T18:42:26Z

Hello,

Thanks for your feedback. Lets again go through them one by one.

NV would make a change to allow a PCD based skipping of CRC check in EEPROM. At your end, you are also going to check with Framos about making sure they are doing CRC updates in EEPROM data. I think we can close this one.
Regarding the phandle-based check in the driver, I meant checking in the I2C device node to see if there is a phandle that would be there only for those EEPROMs that are mentioned in eeprom-manager. From a purely UEFI perspective, we are talking to only EEPROMs in UEFI while the kernel could be talking to other clients on the I2C bus as well.
If you want to add code to NV UEFI for enabling a particular I2C muxer as you have means to test it as well, please feel free and I would accept that and release it to the main branch as part of the next release.
As I summarized in the earlier comment, I do agree that you have a valid use case that was not considered when we designed the overlay application mechanism. So far, we have not had a case where on any platform (in its default state and I agree this may not be the state in which all users may be using the devices) we release reference firmware for, we had more than 1 EEPROM with the same data anywhere on the platform. As I said in point 2 above, the kernel may be considering a wide range of I2C clients listed as child nodes of the controller node but because UEFI only supports a very limited number of them and EEPROMs especially are processed only from eeprom-manager. I am going to think more about this and how to best handle this.

I would get back to you when I have more on 4. Meanwhile, please feel free to provide a commit for 3 as and when you have it.

Thanks
Ashish

eh-steve · 2023-05-18T19:16:25Z

Are you against my suggestion for using a device tree property instead of a PCD? This would avoid the need for users to build a custom UEFI to support their camera...
I still don't understand what you mean here... The presence of an eeprom in /eeprom-manager only adds a phandle to the bus (root) node it references under i2c-bus, but doesn't add phandles for any of the device nodes? Have I misunderstood what you're saying? Unless you mean skipping adding slaves on all I2C controllers that don't have phandles? This would be slightly unexpected behaviour...?
Great, thanks, I'll sort that. I don't currently have a TCA9546A/TCA9548A I can test with, but I'll see what I can do.

we have not had a case where on any platform we release reference firmware for we had more than 1 EEPROM with the same data anywhere on the platform.

Surely the reference e3333 interposer with 6x ov5693s would have 6 identical EEPROMS for the cameras?

EEPROMs especially are processed only from eeprom-manager

You keep saying this, but that isn't true! This code has been present since the initial commit...

ashishsingha · 2023-05-18T21:48:36Z

Let me reply issue by issue:

Issue 1. Both approaches yield the same result. I would define a property /firmware/uefi/skip-eeprom-crc in DTB which can be used to skip CRC check for all EEPROMs.
Issue 2: I apologize I was not able to understand the issue you raised. Regarding the DeviceIndex we use when adding an I2C device, I see that happening in two ways. I see it being done as part of looking at eeprom-manager where we DeviceIndex basically is an incrementing counter, and via supported child nodes in I2C device node. Your concern is with the I2C controller not having a phandle in which case the DeviceIndex will not be unique for every device. I would look into how to resolve such issues including the suggestion from you. If you want your suggestion which is a combination of a counter and controller ID, please create a commit specific for that and nothing else. This would allow me to review and decide on each issue independently.
Issue 3: You would be providing the verified commit for this I2C Muxer.
Issue 4: I would work with the platform team and see how to make enhancements for the e3333 use case where this is indeed an issue. Thanks for helping identify this. If you can split your commit into multiple ones so that I can look at just the proposed solution for this, it would be helpful.

I would look at the issues at appropriate priority and post a fix as and when available. If you want to help out expedite this, please feel free and post targeted commits for each of the issues. Appreciate your help on this.

Thanks
Ashish

eh-steve · 2023-05-18T22:02:54Z

Sure, I'll split these into separate commits 🙂

ashishsingha · 2023-05-19T15:42:25Z

Thank You. Please try to have the commit message in the following format so that I can use it as is. Also, please add signed-off-by tag as well.

fix/chore/feat: title

description

ashishsingha · 2023-05-19T18:53:55Z

Thank You. I will review these in a few days. I will need to rewrite the commit message to follow the NV format more closely.

eh-steve · 2023-05-19T19:07:40Z

As a side question, do you guys have a .clang-format file or something that you use for this repo? I've been fighting my IDE to get indentation the same as the existing codebase and noticed a few tab/space mishaps slipped through

ashishsingha · 2023-05-19T19:08:59Z

We use uncrustify as in https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Code-Formatting

ashishsingha · 2023-05-19T20:02:51Z

Hello,

Please update the commit message to have the fix/feat/chore in small case. The title should be approximately 80 characters and the description say 100 in one line but you can have as many lines as you want.

Also, we would need a legit email address so that it is clear that someone is contributing the code with the applicable license.

Thanks
Ashish

eh-steve · 2023-05-19T20:05:41Z

FYI, I just verified the final result of this branch with my rewritten commits (I changed a few details on the way), on my board using PCA9547, using a base DT that looks like:

/ {
	i2c@3180000 {
		pca9547@70 {
			compatible = "nxp,pca9547";
			reg=<0x70>;
			i2c@0 {
				reg = <0>;
				eeprom@55 {
					compatible = "atmel,24c02";
					reg = <0x55>;
					uefi-skip-crc;
				};
				eeprom@56 {
					compatible = "atmel,24c02";
					reg = <0x56>;
					uefi-skip-crc;
				};
				...
			};
			...
			i2c@5 {
				reg = <5>;
				eeprom@55 {
					compatible = "atmel,24c02";
					reg = <0x55>;
					uefi-skip-crc;
				};
				eeprom@56 {
					compatible = "atmel,24c02";
					reg = <0x56>;
					uefi-skip-crc;
				};
				...
			};
		};
	};
};

And overlays that look like:

	fragment@0 {
		target-path = "/i2c@3180000/pca9547@70/i2c@0/imx462_a@1a";
		board_config {
			ids = "framos-imx462-0";
			eeprom-dt-paths = "/i2c@3180000/pca9547@70/i2c@0/eeprom@55", "/i2c@3180000/pca9547@70/i2c@0/eeprom@56";
			sw-modules = "kernel";
		};
		__overlay__ {
			status = "okay";
		};
	};

And it's all working fine 🎉

eh-steve · 2023-05-19T22:03:28Z

Please update the commit message to have the fix/feat/chore in small case. The title should be approximately 80 characters and the description say 100 in one line but you can have as many lines as you want.

Also, we would need a legit email address so that it is clear that someone is contributing the code with the applicable license.

How about now?

…ly unique This could (and did) happen in the following cases: * the device tree node had no phandle (so would default to 0, which could collide with other devices which also don't have a phandle) * or if the device tree phandle address happened to be the same as the Count of EEPROM devices * or if an atmel compatible EEPROM was defined directly under the i2c bus, this `Count` could (incorrectly) collide with the `Count` used for /eeprom-manager defined devices Instead of using a mix of phandles and Counts, this commit switches to using ((ControllerID << 16) | NumberOfI2cDevices), which should be globally unique across all buses Signed-off-by: eh-steve <[email protected]>

…_PROTOCOL Previously I2C devices behind a mux or switch were not visible to UEFI, and camera EEPROMs behind a mux would not get probed during boot. This commit adds support for all pca954x type I2C mux/switches, which the TCA9548A on the reference e3333 interposer board is compatible with. This allows the device tree to define an I2c mux using a `compatible = "nxp,pca9547"` prop, with child nodes inside, and for UEFI to be able to read camera EEPROMS behind these muxes. It does not yet add support for "i2c-mux-gpio" type muxes, but could potentially be added later. Signed-off-by: eh-steve <[email protected]>

To allow I2cIo drivers (and any other I2C driver) to lookup device tree nodes using DeviceGUID + DeviceIndex. This allows I2C device drivers to get more information about their hardware (e.g. full device tree paths of I2C devices, and read device tree properties) Signed-off-by: eh-steve <[email protected]>

… property NV UEFI currently skips the CRC check for Leopard camera eeproms whose product ID data starts with "LPRD" via a hardcoded exception. Other camera vendors may or may not set the last byte to the CRC8 value, so this exposes a device tree property "uefi-skip-crc" to be added to the eeprom device tree node, so hardcoded exceptions can be gradually removed and moved to device tree instead. This enables Framos camera support Signed-off-by: eh-steve <[email protected]>

…paths" Adds support for specific overlay triggers based on a combination of EEPROM IDs and the exact device tree paths of those EEPROMS. The current behaviour of NV UEFI is that if any EEPROM product ID from any I2C bus matches any of the ids listed in board_config, the overlay is triggered. Effectively all EEPROMs add to a global list of Board IDs, regardless of which device they actually represent. This means a single camera plugged into a single port would make the whole board be treated as matching that camera. This is a problem, since: * it prevents plugging different camera models into different ports and have them work without re-flashing the whole device (any CSI camera port change requires a reflash, instead of supporting various models automatically, and having the interposer board partly populated) * Causes the kernel to create devnodes for devices that don't exist, (causing preventable error logs in nvargus-daemon) This PR adds support for a new, more controlled overlay behaviour via the new (optional) eeprom-dt-paths board_config property, where an overlay can be triggered based on a match of the ID in the EEPROM and a match of the exact I2C device tree path the EEPROM product ID came from. (Old behaviour with existing board_configs should be unchanged). This means that the presence of a single camera no longer adds this Product ID to the global list of board IDs, but that you can selectively apply an overlay only when a specific ID was read from a specific device tree path (i.e. individual CSI cameras can be enumerated during boot and the kernel never creates devnodes for missing devices). ``` board_config { ids = "framos-imx462-0"; eeprom-dt-paths = "/i2c@3180000/pca9547@70/i2c@1/eeprom@55", "/i2c@3180000/pca9547@70/i2c@1/eeprom@56"; sw-modules = "kernel"; }; ``` Signed-off-by: eh-steve <[email protected]>

eh-steve · 2023-05-31T12:22:54Z

Also should this be based on r35.3.1-updates or main?

ashishsingha · 2023-06-01T00:45:25Z

Sorry for the delay in absorbing your patches. I would definitely get to it in the second week of June. Your patches are on main branch which is what we want.

eh-steve · 2023-06-26T06:29:51Z

Hi, just checking in on this - did you get a chance to test it on the e3333?

eh-steve · 2023-07-31T17:43:57Z

Hi, did you get a chance to review these changes?

ashishsingha · 2023-07-31T18:40:23Z

Hi Steve. Sorry for the delay. We have been busy with some other release milestones and would get to this in due time. Again, sorry for the delay.

eh-steve · 2023-07-31T19:36:19Z

No worries 🙂 and thanks for getting back to me!

eh-steve · 2024-02-12T17:10:58Z

Hello, just bumping this again

Signed-off-by: eh-steve <[email protected]>

eh-steve force-pushed the i2cmux-and-eeprom-device-tree-path-support branch from be81360 to f646940 Compare May 18, 2023 01:49

eh-steve force-pushed the i2cmux-and-eeprom-device-tree-path-support branch from a62a2b0 to 4c1473e Compare May 19, 2023 18:46

eh-steve force-pushed the i2cmux-and-eeprom-device-tree-path-support branch from 4c1473e to c39673b Compare May 19, 2023 19:57

eh-steve force-pushed the i2cmux-and-eeprom-device-tree-path-support branch from c39673b to 807d067 Compare May 19, 2023 22:02

eh-steve added 5 commits May 20, 2023 00:46

eh-steve force-pushed the i2cmux-and-eeprom-device-tree-path-support branch from 807d067 to badb4fb Compare May 19, 2023 23:49

Merge branch 'main' into i2cmux-and-eeprom-device-tree-path-support

d62523b

Signed-off-by: eh-steve <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support for per-camera (or per EEPROM) DTB overlays #53

Support for per-camera (or per EEPROM) DTB overlays #53

eh-steve commented May 18, 2023 •

edited

Loading

ashishsingha commented May 18, 2023

eh-steve commented May 18, 2023 •

edited

Loading

eh-steve commented May 18, 2023

ashishsingha commented May 18, 2023

eh-steve commented May 18, 2023 •

edited

Loading

ashishsingha commented May 18, 2023

eh-steve commented May 18, 2023 •

edited

Loading

ashishsingha commented May 18, 2023

eh-steve commented May 18, 2023

ashishsingha commented May 19, 2023

ashishsingha commented May 19, 2023

eh-steve commented May 19, 2023

ashishsingha commented May 19, 2023

ashishsingha commented May 19, 2023

eh-steve commented May 19, 2023 •

edited

Loading

eh-steve commented May 19, 2023

eh-steve commented May 31, 2023

ashishsingha commented Jun 1, 2023

eh-steve commented Jun 26, 2023

eh-steve commented Jul 31, 2023

ashishsingha commented Jul 31, 2023

eh-steve commented Jul 31, 2023

eh-steve commented Feb 12, 2024

Support for per-camera (or per EEPROM) DTB overlays #53

Are you sure you want to change the base?

Support for per-camera (or per EEPROM) DTB overlays #53

Conversation

eh-steve commented May 18, 2023 • edited Loading

ashishsingha commented May 18, 2023

eh-steve commented May 18, 2023 • edited Loading

eh-steve commented May 18, 2023

ashishsingha commented May 18, 2023

eh-steve commented May 18, 2023 • edited Loading

ashishsingha commented May 18, 2023

eh-steve commented May 18, 2023 • edited Loading

ashishsingha commented May 18, 2023

eh-steve commented May 18, 2023

ashishsingha commented May 19, 2023

ashishsingha commented May 19, 2023

eh-steve commented May 19, 2023

ashishsingha commented May 19, 2023

ashishsingha commented May 19, 2023

eh-steve commented May 19, 2023 • edited Loading

eh-steve commented May 19, 2023

eh-steve commented May 31, 2023

ashishsingha commented Jun 1, 2023

eh-steve commented Jun 26, 2023

eh-steve commented Jul 31, 2023

ashishsingha commented Jul 31, 2023

eh-steve commented Jul 31, 2023

eh-steve commented Feb 12, 2024

eh-steve commented May 18, 2023 •

edited

Loading

eh-steve commented May 18, 2023 •

edited

Loading

eh-steve commented May 18, 2023 •

edited

Loading

eh-steve commented May 18, 2023 •

edited

Loading

eh-steve commented May 19, 2023 •

edited

Loading