Hi,

On 2023/11/15 00:30, Dmitry Baryshkov wrote:
+
+               ctx->connector = connector;
+       }

         if (ctx->info->id == ID_IT66121) {
                 ret = regmap_write_bits(ctx->regmap, IT66121_CLK_BANK_REG,
@@ -1632,16 +1651,13 @@ static const char * const it66121_supplies[] = {
         "vcn33", "vcn18", "vrf12"
  };

-static int it66121_probe(struct i2c_client *client)
+int it66121_create_bridge(struct i2c_client *client, bool of_support,
+                         bool hpd_support, bool audio_support,
+                         struct drm_bridge **bridge)
  {
+       struct device *dev = &client->dev;
         int ret;
         struct it66121_ctx *ctx;
-       struct device *dev = &client->dev;
-
-       if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
-               dev_err(dev, "I2C check functionality failed.\n");
-               return -ENXIO;
-       }

         ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
         if (!ctx)
@@ -1649,24 +1665,19 @@ static int it66121_probe(struct i2c_client *client)

         ctx->dev = dev;
         ctx->client = client;
-       ctx->info = i2c_get_match_data(client);
-
-       ret = it66121_of_read_bus_width(dev, &ctx->bus_width);
-       if (ret)
-               return ret;
-
-       ret = it66121_of_get_next_bridge(dev, &ctx->next_bridge);
-       if (ret)
-               return ret;
-
-       i2c_set_clientdata(client, ctx);
         mutex_init(&ctx->lock);

-       ret = devm_regulator_bulk_get_enable(dev, ARRAY_SIZE(it66121_supplies),
-                                            it66121_supplies);
-       if (ret) {
-               dev_err(dev, "Failed to enable power supplies\n");
-               return ret;
+       if (of_support) {
+               ret = it66121_of_read_bus_width(dev, &ctx->bus_width);
+               if (ret)
+                       return ret;
+
+               ret = it66121_of_get_next_bridge(dev, &ctx->next_bridge);
+               if (ret)
+                       return ret;
+       } else {
+               ctx->bus_width = 24;
+               ctx->next_bridge = NULL;
         }
A better alternative would be to turn OF calls into fwnode calls and
to populate the fwnode properties. See
drivers/platform/x86/intel/chtwc_int33fe.c for example.


Honestly, I don't want to leave any scratch(breadcrumbs).
I'm worries about that turn OF calls into fwnode calls will leave something 
unwanted.

Because I am not sure if fwnode calls will make sense in the DT world, while my 
patch
*still* be useful in the DT world. Because the newly introduced 
it66121_create_bridge()
function is a core. I think It's better leave this task to a more advance 
programmer.
if there have use case. It can be introduced at a latter time, probably 
parallel with
the DT.

I think DT and/or ACPI is best for integrated devices, but it66121 display 
bridges is
a i2c slave device. Personally, I think slave device shouldn't be standalone. 
I'm more
prefer to turn this driver to support hot-plug, even remove the device on the 
run time
freely when detach and allow reattach. Like the I2C EEPROM device in the 
monitor (which
contains the EDID, with I2C slave address 0x50). The I2C EEPROM device *also* 
don't has
a corresponding struct device representation in linux kernel.

so I still think It is best to make this drivers functional as a static lib, 
but I want
to hear you to say more. Why it would be a *better* alternative to turn OF 
calls into
fwnode calls? what are the potential benefits?



Reply via email to