@samitouri / QOSamiQemu / commits / 0d02f7b877

hw/gpio: pca9554: add hw-dir property honoring the configured pin direction

The pinN QOM accessors are meant to let external agents observe and stimulate the expander's pins, but their default behaviour does not match real hardware: - to "drive" a pin, set_pin writes the OUTPUT register and then clears the pin's Configuration bit to force it into output mode. On a real device the pin direction is owned solely by the host (programmed through the Configuration register over I2C); an external agent can neither flip a pin's direction nor impose a level on a pin the host drives as an output -- the latter is a voltage conflict, not a legal operation. - get_pin returns a CONFIG|OUTPUT composite, i.e. the guest's intent, rather than the level actually sampled on the pin. The PCA9555 GPIO variant (hw/gpio/pca9552.c) already models this correctly and unconditionally: only input-configured pins can be driven from outside, and reads return the sampled INPUT register. Add a "hw-dir" property to bring the pca9554 pin accessors in line with the hardware (and with the PCA9555 model), without changing the behaviour seen by existing users: - hw-dir=true: set_pin only drives pins the guest has configured as inputs; a set on an output pin is refused with a LOG_UNIMP warning. get_pin returns the sampled INPUT register. - hw-dir=false (default): keeps the legacy, non-conformant behaviour for backward compatibility. Signed-off-by: Emmanuel Blot <emmanuel.blot@free.fr> Reviewed-by: Glenn Miles <milesg@linux.ibm.com> Link: https://lore.kernel.org/qemu-devel/20260709-catalina-upgrade-v1-16-82a63fead90c@free.fr Signed-off-by: Cédric Le Goater <clg@redhat.com>

Emmanuel Blot committed Jul 9, 2026 at 17:23 UTC 0d02f7b87789f95393dcaa4b9109e29a061ae120

2 files changed +43 -23

hw/gpio/pca9554.c

+42 -23

@@ -154,6 +154,14 @@ static int pca9554_event(I2CSlave *i2c, enum i2c_event event)

154 return 0;

155 }

156

157 +static void pca9554_set_ext_state(PCA9554State *s, int pin, int level)

158 +{

159 + if (s->ext_state[pin] != level) {

160 + s->ext_state[pin] = level;

161 + pca9554_update_pin_input(s);

162 + }

163 +}

164 +

165 static void pca9554_get_pin(Object *obj, Visitor *v, const char *name,

166 void *opaque, Error **errp)

167 {

@@ -171,9 +179,13 @@ static void pca9554_get_pin(Object *obj, Visitor *v, const char *name,

179 return;

180 }

181

174 - state = pca9554_read(s, PCA9554_CONFIG);

175 - state |= pca9554_read(s, PCA9554_OUTPUT);

176 - state = (state >> pin) & 0x1;

182 + /*

183 + * Report the physical pin level. The input register is kept in sync by

184 + * pca9554_update_pin_input(): output pins mirror the OUTPUT register and

185 + * input pins reflect the externally driven (or pulled-up) level, so it

186 + * holds the wire level regardless of the configured direction.

187 + */

188 + state = (s->regs[PCA9554_INPUT] >> pin) & 0x1;

189 visit_type_str(v, name, (char **)&pin_state[state], errp);

190 }

191

@@ -208,20 +220,34 @@ static void pca9554_set_pin(Object *obj, Visitor *v, const char *name,

220 return;

221 }

222

211 - /* First, modify the output register bit */

212 - val = pca9554_read(s, PCA9554_OUTPUT);

213 - mask = 0x1 << pin;

214 - if (state == PCA9554_PIN_LOW) {

215 - val &= ~(mask);

223 + if (s->hw_dir) {

224 + /* Warn and ignore if the guest has configured this pin as output */

225 + if (!((s->regs[PCA9554_CONFIG] >> pin) & 0x1)) {

226 + qemu_log_mask(LOG_UNIMP,

227 + "%s: pin %d is configured as output, "

228 + "ignoring external set\n",

229 + s->description, pin);

230 + return;

231 + }

232 + /* Drive the external input level */

233 + pca9554_set_ext_state(s, pin, state != PCA9554_PIN_LOW);

234 } else {

217 - val |= mask;

218 - }

219 - pca9554_write(s, PCA9554_OUTPUT, val);

235 + /* Legacy behavior: force output mode and drive */

236 + /* First, modify the output register bit */

237 + val = pca9554_read(s, PCA9554_OUTPUT);

238 + mask = 0x1 << pin;

239 + if (state == PCA9554_PIN_LOW) {

240 + val &= ~(mask);

241 + } else {

242 + val |= mask;

243 + }

244 + pca9554_write(s, PCA9554_OUTPUT, val);

245

221 - /* Then, clear the config register bit for output mode */

222 - val = pca9554_read(s, PCA9554_CONFIG);

223 - val &= ~mask;

224 - pca9554_write(s, PCA9554_CONFIG, val);

246 + /* Then, clear the config register bit for output mode */

247 + val = pca9554_read(s, PCA9554_CONFIG);

248 + val &= ~mask;

249 + pca9554_write(s, PCA9554_CONFIG, val);

250 + }

251 }

252

253 static const VMStateDescription pca9554_vmstate = {

@@ -271,14 +297,6 @@ static void pca9554_initfn(Object *obj)

297 }

298 }

299

274 -static void pca9554_set_ext_state(PCA9554State *s, int pin, int level)

275 -{

276 - if (s->ext_state[pin] != level) {

277 - s->ext_state[pin] = level;

278 - pca9554_update_pin_input(s);

279 - }

280 -}

281 -

300 static void pca9554_gpio_in_handler(void *opaque, int pin, int level)

301 {

302 PCA9554State *s = PCA9554(opaque);

@@ -303,6 +321,7 @@ static void pca9554_realize(DeviceState *dev, Error **errp)

321

322 static const Property pca9554_properties[] = {

323 DEFINE_PROP_STRING("description", PCA9554State, description),

324 + DEFINE_PROP_BOOL("hw-dir", PCA9554State, hw_dir, false),

325 };

326

327 static void pca9554_class_init(ObjectClass *klass, const void *data)

include/hw/gpio/pca9554.h

@@ -33,6 +33,7 @@ struct PCA9554State {

33 qemu_irq gpio_out[PCA9554_PIN_COUNT];

34 uint8_t ext_state[PCA9554_PIN_COUNT];

35 char *description; /* For debugging purpose only */

36 + bool hw_dir; /* Honor pin direction */

37 };

39 #endif